> On Nov. 12, 2015, 1:16 p.m., Bernd Mathiske wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 130 > > <https://reviews.apache.org/r/39594/diff/5/?file=1121773#file1121773line130> > > > > Strictly speaking, "file" is pushing assumptions into this. > > modificationTime? mTime? mtime?
Since I `s/fileSize/contentLength/g` I also went for `s/fileTime/contentLength/g` to match the HTTP headers. > On Nov. 12, 2015, 1:16 p.m., Bernd Mathiske wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 88 > > <https://reviews.apache.org/r/39594/diff/5/?file=1121773#file1121773line88> > > > > Since we are only dealing with URLs here, not general URIs, how about > > "URLInfo"? I definitely like the more compact name! Since this struct isn't really specific to URLs I think making the name not URL-specific as well would make sense; this patch chain e.g. uses it in mesos to capture info on various kinds of URI. How about `URIInfo`? > On Nov. 12, 2015, 1:16 p.m., Bernd Mathiske wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 89 > > <https://reviews.apache.org/r/39594/diff/5/?file=1121773#file1121773line89> > > > > 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"? Cf. comment Re:`struct URIInformation`. > On Nov. 12, 2015, 1:16 p.m., Bernd Mathiske wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/fs.hpp, line 45 > > <https://reviews.apache.org/r/39594/diff/5/?file=1121772#file1121772line45> > > > > 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. Good catch, `fs::mtime` does not make much sense as written. How about synchronizing the signatures and default args of `os::stat::size` and `os::stat::mtime` so that users can decide what they wanted? - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39594/#review106236 ----------------------------------------------------------- On Nov. 13, 2015, 12:16 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39594/ > ----------------------------------------------------------- > > (Updated Nov. 13, 2015, 12:16 p.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]: Extended os::stat::mtime to optionally follow links. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp > e49783a438157706b1be9745436bf666f45cab8b > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/stat.hpp > 6b1f7ad70a9cac9352463006c514e637f68e66ad > > Diff: https://reviews.apache.org/r/39594/diff/ > > > Testing > ------- > > make check > > > NOTE TO THE COMMITTER: > > This should be committed together with RR-40288 (first this, then that one) > to not change semantics. > > > Thanks, > > Benjamin Bannier > >
