Re: Review Request 50907: Abstracted mesos-style.py to wrap the cpp linter in a class.

[Kevin Klues]

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

(Updated Aug. 11, 2016, 6:26 p.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Changes
---

Addressed Joseph's comments.


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


Repository: mesos


Description
---

Previously, mesos-style.py was just a collection of functions that
checked the style of relevant files in the mesos code base.  However,
the script assumed that we always wanted to run cpplint over every
file we were checking. Since we are planning on adding a python linter
to the codebase soon, it makes sense to abstract the common
functionality from this script into a class so that a cpp-based linter
and a python-based linter can inherit the same set of common
functionality.

This commit builds this abstraction and implements a 'CppLinter()' in
terms of it.


Diffs (updated)
-

  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---

Ran `support/mesos-style.py` over the whole code base.


Thanks,

Kevin Klues

Re: Review Request 50907: Abstracted mesos-style.py to wrap the cpp linter in a class.

[Joseph Wu]

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


Fix it, then Ship it!




Since you're making the linter object into some sort of interface, you should 
minimally add comments for object and override-able functions.


support/mesos-style.py (lines 11 - 12)


Would be nice to have a comment here, describing:

* General function of the class
* What fields/functions to override



support/mesos-style.py (lines 43 - 44)


Would be nice to have a comment here, describing:

* When this is called
* What the expected arguments are


- Joseph Wu


On Aug. 8, 2016, 12:49 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50907/
> ---
> 
> (Updated Aug. 8, 2016, 12:49 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-6006
> https://issues.apache.org/jira/browse/MESOS-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, mesos-style.py was just a collection of functions that
> checked the style of relevant files in the mesos code base.  However,
> the script assumed that we always wanted to run cpplint over every
> file we were checking. Since we are planning on adding a python linter
> to the codebase soon, it makes sense to abstract the common
> functionality from this script into a class so that a cpp-based linter
> and a python-based linter can inherit the same set of common
> functionality.
> 
> This commit builds this abstraction and implements a 'CppLinter()' in
> terms of it.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50907/diff/
> 
> 
> Testing
> ---
> 
> Ran `support/mesos-style.py` over the whole code base.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 50907: Abstracted mesos-style.py to wrap the cpp linter in a class.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Aug. 8, 2016, 7:49 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50907/
> ---
> 
> (Updated Aug. 8, 2016, 7:49 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-6006
> https://issues.apache.org/jira/browse/MESOS-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, mesos-style.py was just a collection of functions that
> checked the style of relevant files in the mesos code base.  However,
> the script assumed that we always wanted to run cpplint over every
> file we were checking. Since we are planning on adding a python linter
> to the codebase soon, it makes sense to abstract the common
> functionality from this script into a class so that a cpp-based linter
> and a python-based linter can inherit the same set of common
> functionality.
> 
> This commit builds this abstraction and implements a 'CppLinter()' in
> terms of it.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50907/diff/
> 
> 
> Testing
> ---
> 
> Ran `support/mesos-style.py` over the whole code base.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 50907: Abstracted mesos-style.py to wrap the cpp linter in a class.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50907]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Aug. 8, 2016, 7:49 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50907/
> ---
> 
> (Updated Aug. 8, 2016, 7:49 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-6006
> https://issues.apache.org/jira/browse/MESOS-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, mesos-style.py was just a collection of functions that
> checked the style of relevant files in the mesos code base.  However,
> the script assumed that we always wanted to run cpplint over every
> file we were checking. Since we are planning on adding a python linter
> to the codebase soon, it makes sense to abstract the common
> functionality from this script into a class so that a cpp-based linter
> and a python-based linter can inherit the same set of common
> functionality.
> 
> This commit builds this abstraction and implements a 'CppLinter()' in
> terms of it.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50907/diff/
> 
> 
> Testing
> ---
> 
> Ran `support/mesos-style.py` over the whole code base.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 50907: Abstracted mesos-style.py to wrap the cpp linter in a class.

[Kevin Klues]

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

(Updated Aug. 8, 2016, 7:49 p.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Changes
---

Added the JIRA link


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


Repository: mesos


Description
---

Previously, mesos-style.py was just a collection of functions that
checked the style of relevant files in the mesos code base.  However,
the script assumed that we always wanted to run cpplint over every
file we were checking. Since we are planning on adding a python linter
to the codebase soon, it makes sense to abstract the common
functionality from this script into a class so that a cpp-based linter
and a python-based linter can inherit the same set of common
functionality.

This commit builds this abstraction and implements a 'CppLinter()' in
terms of it.


Diffs
-

  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---

Ran `support/mesos-style.py` over the whole code base.


Thanks,

Kevin Klues

Review Request 50907: Abstracted mesos-style.py to wrap the cpp linter in a class.

[Kevin Klues]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

Previously, mesos-style.py was just a collection of functions that
checked the style of relevant files in the mesos code base.  However,
the script assumed that we always wanted to run cpplint over every
file we were checking. Since we are planning on adding a python linter
to the codebase soon, it makes sense to abstract the common
functionality from this script into a class so that a cpp-based linter
and a python-based linter can inherit the same set of common
functionality.

This commit builds this abstraction and implements a 'CppLinter()' in
terms of it.


Diffs
-

  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---

Ran `support/mesos-style.py` over the whole code base.


Thanks,

Kevin Klues