Re: Review Request 39449: Documented order of includes.

2015-11-23 Thread Michael Park 

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

Ship it!


Ship It!

- Michael Park


On Nov. 20, 2015, 11:17 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39449/
> ---
> 
> (Updated Nov. 20, 2015, 11:17 a.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Michael Park.
> 
> 
> Bugs: MESOS-2275
> https://issues.apache.org/jira/browse/MESOS-2275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented order of includes.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/39449/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39449: Documented order of includes.

2015-11-20 Thread Jan Schlicht 

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

(Updated Nov. 20, 2015, 12:17 p.m.)


Review request for mesos, Marco Massenzio and Michael Park.


Changes
---

Related headers should be included first.


Bugs: MESOS-2275
https://issues.apache.org/jira/browse/MESOS-2275


Repository: mesos


Description
---

Documented order of includes.


Diffs (updated)
-

  docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 

Diff: https://reviews.apache.org/r/39449/diff/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 39449: Documented order of includes.

2015-11-20 Thread Jan Schlicht 


> On Nov. 3, 2015, 10:53 p.m., Joerg Schad wrote:
> > docs/c++-style-guide.md, line 251
> > 
> >
> > Could we add a short comment above every new section describing the 
> > representative meaning of each? (e.g. here nested subfolder)

IMHO this could get quite redundant if we do this for every source file.


- Jan


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


On Oct. 19, 2015, 11:29 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39449/
> ---
> 
> (Updated Oct. 19, 2015, 11:29 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Michael Park.
> 
> 
> Bugs: MESOS-2275
> https://issues.apache.org/jira/browse/MESOS-2275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented order of includes.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/39449/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39449: Documented order of includes.

2015-11-20 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [39449]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 20, 2015, 11:17 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39449/
> ---
> 
> (Updated Nov. 20, 2015, 11:17 a.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Michael Park.
> 
> 
> Bugs: MESOS-2275
> https://issues.apache.org/jira/browse/MESOS-2275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented order of includes.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/39449/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39449: Documented order of includes.

2015-11-03 Thread Joerg Schad 

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



docs/c++-style-guide.md (line 262)


This should be the first include (see my other comments and the Google 
Styleguide ( 
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes) 
:-)).


- Joerg Schad


On Oct. 19, 2015, 9:29 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39449/
> ---
> 
> (Updated Oct. 19, 2015, 9:29 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Michael Park.
> 
> 
> Bugs: MESOS-2275
> https://issues.apache.org/jira/browse/MESOS-2275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented order of includes.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/39449/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39449: Documented order of includes.

2015-11-03 Thread Joerg Schad 

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

Ship it!



docs/c++-style-guide.md (line 235)


Could we add that for cpp files the repective .h file should be included 
first (which is specified in the Google Styleguide but many files ignore this)



docs/c++-style-guide.md (lines 240 - 241)


Add foo.hpp as first include.



docs/c++-style-guide.md (line 251)


Could we add a short comment above every new section describing the 
representative meaning of each? (e.g. here nested subfolder)


- Joerg Schad


On Oct. 19, 2015, 9:29 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39449/
> ---
> 
> (Updated Oct. 19, 2015, 9:29 p.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Michael Park.
> 
> 
> Bugs: MESOS-2275
> https://issues.apache.org/jira/browse/MESOS-2275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented order of includes.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/39449/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39449: Documented order of includes.

2015-10-24 Thread Qian Zhang 

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

Ship it!


Ship It!

- Qian Zhang


On Oct. 20, 2015, 5:29 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39449/
> ---
> 
> (Updated Oct. 20, 2015, 5:29 a.m.)
> 
> 
> Review request for mesos, Marco Massenzio and Michael Park.
> 
> 
> Bugs: MESOS-2275
> https://issues.apache.org/jira/browse/MESOS-2275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented order of includes.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/39449/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39449: Documented order of includes.

2015-10-19 Thread Jan Schlicht 


> On Oct. 19, 2015, 9:13 p.m., Marco Massenzio wrote:
> > docs/c++-style-guide.md, line 243
> > 
> >
> > I would specify what .ccp file is this example related to.
> > 
> > I think we have the 'related header' rule, where, if this where 
> > `master/flags.cpp` then `master/flags.hpp` should be included first.

Currently the related headers are not included first (contrary to the Google 
Style Doc), will add this behavior to the doc.


- Jan


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


On Oct. 19, 2015, 8:58 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39449/
> ---
> 
> (Updated Oct. 19, 2015, 8:58 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-2275
> https://issues.apache.org/jira/browse/MESOS-2275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented order of includes.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/39449/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39449: Documented order of includes.

2015-10-19 Thread Jan Schlicht 

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

(Updated Oct. 19, 2015, 11:20 p.m.)


Review request for mesos and Michael Park.


Changes
---

Addressed issues. Headers are included alphabetically.


Bugs: MESOS-2275
https://issues.apache.org/jira/browse/MESOS-2275


Repository: mesos


Description
---

Documented order of includes.


Diffs (updated)
-

  docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 

Diff: https://reviews.apache.org/r/39449/diff/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 39449: Documented order of includes.

2015-10-19 Thread Marco Massenzio 

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



docs/c++-style-guide.md (line 241)


s/seperate/separate



docs/c++-style-guide.md (line 243)


I would specify what .ccp file is this example related to.

I think we have the 'related header' rule, where, if this where 
`master/flags.cpp` then `master/flags.hpp` should be included first.


- Marco Massenzio


On Oct. 19, 2015, 6:58 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39449/
> ---
> 
> (Updated Oct. 19, 2015, 6:58 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-2275
> https://issues.apache.org/jira/browse/MESOS-2275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented order of includes.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 0b6189174a4f0f1815625f68fb1a743b04a9cdad 
> 
> Diff: https://reviews.apache.org/r/39449/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>