mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bernd Mathiske" <be...@mesosphere.io>
Subject Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.
Date Thu, 12 Nov 2015 13:16:58 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39594/#review106236
-----------------------------------------------------------



3rdparty/libprocess/3rdparty/stout/include/stout/fs.hpp (line 45)
<https://reviews.apache.org/r/39594/#comment164973>

    This already exists in "stat.hpp", but that version uses lstat() and we really want stat().
See size() in "stat.hpp"! Let's make a version of mtime that looks like this!
    
    There is only one other callsite of mtime() so far and we should find out whether following
symlinks is appropriate there. To be backwards-compatible, we should not.



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp (line 88)
<https://reviews.apache.org/r/39594/#comment164976>

    Since we are only dealing with URLs here, not general URIs, how about "URLInfo"?



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp (line 89)
<https://reviews.apache.org/r/39594/#comment164981>

    Since the name of the header field that feeds into this field is content-length, we should
probably use a name like that here. How about "contentLength"?



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp (line 99)
<https://reviews.apache.org/r/39594/#comment164974>

    Naming suggestion: fetchURLInfo()



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp (line 124)
<https://reviews.apache.org/r/39594/#comment164985>

    s/fileSize/contentLength
    
    Let's interprete what this header field actually means to us further downstream.



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp (line 130)
<https://reviews.apache.org/r/39594/#comment164987>

    Strictly speaking, "file" is pushing assumptions into this. modificationTime? mTime? mtime?


- Bernd Mathiske


On Nov. 10, 2015, 6:07 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39594/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2015, 6:07 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3785
>     https://issues.apache.org/jira/browse/MESOS-3785
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> [stout]: Added function to simultaneously query size and mtime of URI.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/fs.hpp 311b00b41398a9fd7374f3847190468ba59c1dc9

>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp e49783a438157706b1be9745436bf666f45cab8b

> 
> Diff: https://reviews.apache.org/r/39594/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message