Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Jan Schlicht]

> On Nov. 9, 2015, 2:54 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 141
> > 
> >
> > Why not use `nullptr` instead?

Is `nullptr` okay to use? Judging from MESOS-3243 I'd say that it's not there 
yet. Also there's no mention of it in the C++ style guide.


- Jan


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


On Nov. 6, 2015, 8:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39594/
> ---
> 
> (Updated Nov. 6, 2015, 8:52 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]: 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
> 
>

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Till Toenshoff]

> On Nov. 9, 2015, 1:54 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 141
> > 
> >
> > Why not use `nullptr` instead?
> 
> Jan Schlicht wrote:
> Is `nullptr` okay to use? Judging from MESOS-3243 I'd say that it's not 
> there yet. Also there's no mention of it in the C++ style guide.
> 
> Benjamin Bannier wrote:
> I would have commented that since this file completely relies on `NULL` 
> instead of `nullptr` already consistence would suggest to stick to `NULL`.

I would interpret MESOS-3243 as a transition ticket for old, existing code and 
not for new code.


- Till


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


On Nov. 6, 2015, 7:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39594/
> ---
> 
> (Updated Nov. 6, 2015, 7:52 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]: 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
> 
>

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Benjamin Bannier]

> On Nov. 9, 2015, 1:54 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 141
> > 
> >
> > Why not use `nullptr` instead?
> 
> Jan Schlicht wrote:
> Is `nullptr` okay to use? Judging from MESOS-3243 I'd say that it's not 
> there yet. Also there's no mention of it in the C++ style guide.

I would have commented that since this file completely relies on `NULL` instead 
of `nullptr` already consistence would suggest to stick to `NULL`.


- Benjamin


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


On Nov. 6, 2015, 7:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39594/
> ---
> 
> (Updated Nov. 6, 2015, 7:52 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]: 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
> 
>

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Benjamin Bannier]

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

(Updated Nov. 10, 2015, 2:07 p.m.)


Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.


Changes
---

Improved "jaggedness".


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 (updated)
-

  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

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Jan Schlicht]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp (line 132)


Please add a newline after `=` as you did in l125.


- Jan Schlicht


On Nov. 10, 2015, 2:45 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39594/
> ---
> 
> (Updated Nov. 10, 2015, 2:45 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]: 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
> 
>

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Benjamin Bannier]

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

(Updated Nov. 10, 2015, 1:45 p.m.)


Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.


Changes
---

Incorporated review comments.

* remove unused function `contentLength`,
* fix doxygen comment style,
* use `nullptr` instead of `NULL` in new code, and
* use symbol `CURLE_OK` instead of literal `0`.


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 (updated)
-

  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

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Benjamin Bannier]

> On Nov. 9, 2015, 1:54 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 130
> > 
> >
> > Add blank line here please.

I would be happy to oblige, but that wouldn't commit due cpplint,

Redundant blank line at the start of a code block should be deleted.  
[whitespace/blank_line] [2]


> On Nov. 9, 2015, 1:54 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 141
> > 
> >
> > Why not use `nullptr` instead?
> 
> Jan Schlicht wrote:
> Is `nullptr` okay to use? Judging from MESOS-3243 I'd say that it's not 
> there yet. Also there's no mention of it in the C++ style guide.
> 
> Benjamin Bannier wrote:
> I would have commented that since this file completely relies on `NULL` 
> instead of `nullptr` already consistence would suggest to stick to `NULL`.
> 
> Till Toenshoff wrote:
> I would interpret MESOS-3243 as a transition ticket for old, existing 
> code and not for new code.

No problem, I `nullptr`ized that instance, but left the others for the 
automatic cleanup.


- Benjamin


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


On Nov. 10, 2015, 1:45 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39594/
> ---
> 
> (Updated Nov. 10, 2015, 1:45 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]: 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
> 
>

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Till Toenshoff]

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


As discussed, let's re-iterate on the actual function / signature.


3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp (line 130)


Add blank line here please.



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp (lines 131 - 135)


According to our doxygen-styleguide, we are using C comments;

```
/**
 * 
 * 
 */
```



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp (line 141)


Why not use `nullptr` instead?



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp (line 142)






3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp (line 154)


s/0/CURLE_OK/


- Till Toenshoff


On Nov. 6, 2015, 7:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39594/
> ---
> 
> (Updated Nov. 6, 2015, 7:52 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]: 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
> 
>

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Benjamin Bannier]

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

(Updated Nov. 6, 2015, 11:04 a.m.)


Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.


Changes
---

Moved `SizeAndMtime` to `net.hpp` as suggest in RR-39595.


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 (updated)
-

  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

Re: Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Benjamin Bannier]

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

(Updated Nov. 6, 2015, 7:52 p.m.)


Review request for mesos, Bernd Mathiske, Jan Schlicht, and Till Toenshoff.


Changes
---

Do not `move` to improve readbility.


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 (updated)
-

  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

Review Request 39594: [stout]: Added function to simultaneously query size and mtime of URI.

[Benjamin Bannier]

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

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