Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Joseph Wu]

> On Oct. 4, 2016, 5:18 p.m., Joseph Wu wrote:
> > support/mesos-style.py, lines 270-272
> > 
> >
> > It may help to change the order of reviews (no rebase necessary, just 
> > the "depends on" field).  
> > 
> > Because you add a linter in this review, that depends on the next one ( 
> > https://reviews.apache.org/r/50912/ ), this review fails to lint itself :)
> 
> Kevin Klues wrote:
> I'm confused. The order of these patches (and their dependencies) should 
> be correct. This patch "depends on" 50907, not 50912. Also, when I 
> cherry-pick each of these patches onto master individually, they all get 
> linted just as expected.
> 
> Kevin Klues wrote:
> For example, from master:
> ```
> klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50907
> 2016-10-04 20:31:30 URL:https://reviews.apache.org/r/50907/diff/raw/ 
> [16055/16055] -> "50907.patch" [1]
> No C++ files to lint
> [cli-test d536361] Abstracted mesos-style.py to wrap the cpp linter in a 
> class.
>  1 file changed, 249 insertions(+), 169 deletions(-)
>  rewrite support/mesos-style.py (95%)
> klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50910
> 2016-10-04 20:31:39 URL:https://reviews.apache.org/r/50910/diff/raw/ 
> [2997/2997] -> "50910.patch" [1]
> No C++ files to lint
> No Python files to lint
> [cli-test 2b9502e] Added a python linter to mesos-style.cpp.
>  Author: Haris Choudhary 
>  1 file changed, 67 insertions(+), 2 deletions(-)
> klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50912
> 2016-10-04 20:31:51 URL:https://reviews.apache.org/r/50912/diff/raw/ 
> [40930/40930] -> "50912.patch" [1]
> No C++ files to lint
> Checking 8 Python files
> Total errors found: 0
> [cli-test 88e5726] Added the infrastructure for a new python-based CLI.
>  20 files changed, 1019 insertions(+), 3 deletions(-)
>  create mode 100644 src/cli_new/.gitignore
>  create mode 100644 src/cli_new/README.md
>  create mode 100644 src/cli_new/activate
>  create mode 100644 src/cli_new/bin/config.py
>  create mode 100644 src/cli_new/bin/main.py
>  create mode 100755 src/cli_new/bin/mesos
>  create mode 100755 src/cli_new/bootstrap
>  create mode 100644 src/cli_new/deactivate
>  create mode 100644 src/cli_new/lib/mesos/__init__.py
>  create mode 100644 src/cli_new/lib/mesos/docopt.py
>  create mode 100644 src/cli_new/lib/mesos/exceptions.py
>  create mode 100644 src/cli_new/lib/mesos/plugins/__init__.py
>  create mode 100644 src/cli_new/lib/mesos/plugins/base.py
>  create mode 100644 src/cli_new/lib/mesos/util.py
>  create mode 100644 src/cli_new/mesos.bash_completion
>  create mode 100644 src/cli_new/pip-requirements.txt
>  create mode 100644 src/cli_new/pylint.config
> ```
> 
> Joseph Wu wrote:
> Presumably, that's because your source directory contains leftover 
> directories/files (like `*.pyc` files, which prevent git from deleting the 
> `cli_new` folder):
> ```
> $ support/apply-reviews.py -n -r 50910 -c
> 2016-10-05 10:49:31 URL:https://reviews.apache.org/r/50907/diff/raw/ 
> [16055/16055] -> "50907.patch" [1]
> No C++ files to lint
> [detached HEAD faed65f] Abstracted mesos-style.py to wrap the cpp linter 
> in a class.
>  Author: Kevin Klues 
>  1 file changed, 249 insertions(+), 169 deletions(-)
>  rewrite support/mesos-style.py (95%)
> 2016-10-05 10:49:32 URL:https://reviews.apache.org/r/50910/diff/raw/ 
> [2997/2997] -> "50910.patch" [1]
> No C++ files to lint
> Could not find 'src/cli_new'
> Please run from the root of the mesos source directory
> ```
> 
> If I do:
> ```
> $ support/apply-reviews.py -n -r 50907
> $ support/apply-reviews.py -n -r 50912 # <- reordered this one.
> $ support/apply-reviews.py -n -r 50910
> ```
> 
> It applies cleanly.
> 
> Kevin Klues wrote:
> I see. I think we should then add the linter in this commit (but with an 
> emty array of folders to check). And then introduce this folder into the 
> array in the next commit.  Otherwise we miss out on linting the entire base 
> patch for the CLI.

Hm, probably not worth the trouble of splitting up the reviews.  I'll just do 
this:
```
$ support/apply-reviews.py -n -r 50907
$ mkdir src/cli_new
$ support/apply-reviews.py -n -r 50910  # Applies cleanly now.
```


- Joseph


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


On Aug. 11, 2016, 2:54 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> 

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

> On Oct. 5, 2016, 12:18 a.m., Joseph Wu wrote:
> > support/mesos-style.py, lines 270-272
> > 
> >
> > It may help to change the order of reviews (no rebase necessary, just 
> > the "depends on" field).  
> > 
> > Because you add a linter in this review, that depends on the next one ( 
> > https://reviews.apache.org/r/50912/ ), this review fails to lint itself :)
> 
> Kevin Klues wrote:
> I'm confused. The order of these patches (and their dependencies) should 
> be correct. This patch "depends on" 50907, not 50912. Also, when I 
> cherry-pick each of these patches onto master individually, they all get 
> linted just as expected.
> 
> Kevin Klues wrote:
> For example, from master:
> ```
> klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50907
> 2016-10-04 20:31:30 URL:https://reviews.apache.org/r/50907/diff/raw/ 
> [16055/16055] -> "50907.patch" [1]
> No C++ files to lint
> [cli-test d536361] Abstracted mesos-style.py to wrap the cpp linter in a 
> class.
>  1 file changed, 249 insertions(+), 169 deletions(-)
>  rewrite support/mesos-style.py (95%)
> klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50910
> 2016-10-04 20:31:39 URL:https://reviews.apache.org/r/50910/diff/raw/ 
> [2997/2997] -> "50910.patch" [1]
> No C++ files to lint
> No Python files to lint
> [cli-test 2b9502e] Added a python linter to mesos-style.cpp.
>  Author: Haris Choudhary 
>  1 file changed, 67 insertions(+), 2 deletions(-)
> klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50912
> 2016-10-04 20:31:51 URL:https://reviews.apache.org/r/50912/diff/raw/ 
> [40930/40930] -> "50912.patch" [1]
> No C++ files to lint
> Checking 8 Python files
> Total errors found: 0
> [cli-test 88e5726] Added the infrastructure for a new python-based CLI.
>  20 files changed, 1019 insertions(+), 3 deletions(-)
>  create mode 100644 src/cli_new/.gitignore
>  create mode 100644 src/cli_new/README.md
>  create mode 100644 src/cli_new/activate
>  create mode 100644 src/cli_new/bin/config.py
>  create mode 100644 src/cli_new/bin/main.py
>  create mode 100755 src/cli_new/bin/mesos
>  create mode 100755 src/cli_new/bootstrap
>  create mode 100644 src/cli_new/deactivate
>  create mode 100644 src/cli_new/lib/mesos/__init__.py
>  create mode 100644 src/cli_new/lib/mesos/docopt.py
>  create mode 100644 src/cli_new/lib/mesos/exceptions.py
>  create mode 100644 src/cli_new/lib/mesos/plugins/__init__.py
>  create mode 100644 src/cli_new/lib/mesos/plugins/base.py
>  create mode 100644 src/cli_new/lib/mesos/util.py
>  create mode 100644 src/cli_new/mesos.bash_completion
>  create mode 100644 src/cli_new/pip-requirements.txt
>  create mode 100644 src/cli_new/pylint.config
> ```
> 
> Joseph Wu wrote:
> Presumably, that's because your source directory contains leftover 
> directories/files (like `*.pyc` files, which prevent git from deleting the 
> `cli_new` folder):
> ```
> $ support/apply-reviews.py -n -r 50910 -c
> 2016-10-05 10:49:31 URL:https://reviews.apache.org/r/50907/diff/raw/ 
> [16055/16055] -> "50907.patch" [1]
> No C++ files to lint
> [detached HEAD faed65f] Abstracted mesos-style.py to wrap the cpp linter 
> in a class.
>  Author: Kevin Klues 
>  1 file changed, 249 insertions(+), 169 deletions(-)
>  rewrite support/mesos-style.py (95%)
> 2016-10-05 10:49:32 URL:https://reviews.apache.org/r/50910/diff/raw/ 
> [2997/2997] -> "50910.patch" [1]
> No C++ files to lint
> Could not find 'src/cli_new'
> Please run from the root of the mesos source directory
> ```
> 
> If I do:
> ```
> $ support/apply-reviews.py -n -r 50907
> $ support/apply-reviews.py -n -r 50912 # <- reordered this one.
> $ support/apply-reviews.py -n -r 50910
> ```
> 
> It applies cleanly.

I see. I think we should then add the linter in this commit (but with an emty 
array of folders to check). And then introduce this folder into the array in 
the next commit.  Otherwise we miss out on linting the entire base patch for 
the CLI.


- Kevin


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


On Aug. 11, 2016, 9:54 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 11, 2016, 9:54 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> 

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Joseph Wu]

> On Oct. 4, 2016, 5:18 p.m., Joseph Wu wrote:
> > support/mesos-style.py, lines 270-272
> > 
> >
> > It may help to change the order of reviews (no rebase necessary, just 
> > the "depends on" field).  
> > 
> > Because you add a linter in this review, that depends on the next one ( 
> > https://reviews.apache.org/r/50912/ ), this review fails to lint itself :)
> 
> Kevin Klues wrote:
> I'm confused. The order of these patches (and their dependencies) should 
> be correct. This patch "depends on" 50907, not 50912. Also, when I 
> cherry-pick each of these patches onto master individually, they all get 
> linted just as expected.
> 
> Kevin Klues wrote:
> For example, from master:
> ```
> klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50907
> 2016-10-04 20:31:30 URL:https://reviews.apache.org/r/50907/diff/raw/ 
> [16055/16055] -> "50907.patch" [1]
> No C++ files to lint
> [cli-test d536361] Abstracted mesos-style.py to wrap the cpp linter in a 
> class.
>  1 file changed, 249 insertions(+), 169 deletions(-)
>  rewrite support/mesos-style.py (95%)
> klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50910
> 2016-10-04 20:31:39 URL:https://reviews.apache.org/r/50910/diff/raw/ 
> [2997/2997] -> "50910.patch" [1]
> No C++ files to lint
> No Python files to lint
> [cli-test 2b9502e] Added a python linter to mesos-style.cpp.
>  Author: Haris Choudhary 
>  1 file changed, 67 insertions(+), 2 deletions(-)
> klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50912
> 2016-10-04 20:31:51 URL:https://reviews.apache.org/r/50912/diff/raw/ 
> [40930/40930] -> "50912.patch" [1]
> No C++ files to lint
> Checking 8 Python files
> Total errors found: 0
> [cli-test 88e5726] Added the infrastructure for a new python-based CLI.
>  20 files changed, 1019 insertions(+), 3 deletions(-)
>  create mode 100644 src/cli_new/.gitignore
>  create mode 100644 src/cli_new/README.md
>  create mode 100644 src/cli_new/activate
>  create mode 100644 src/cli_new/bin/config.py
>  create mode 100644 src/cli_new/bin/main.py
>  create mode 100755 src/cli_new/bin/mesos
>  create mode 100755 src/cli_new/bootstrap
>  create mode 100644 src/cli_new/deactivate
>  create mode 100644 src/cli_new/lib/mesos/__init__.py
>  create mode 100644 src/cli_new/lib/mesos/docopt.py
>  create mode 100644 src/cli_new/lib/mesos/exceptions.py
>  create mode 100644 src/cli_new/lib/mesos/plugins/__init__.py
>  create mode 100644 src/cli_new/lib/mesos/plugins/base.py
>  create mode 100644 src/cli_new/lib/mesos/util.py
>  create mode 100644 src/cli_new/mesos.bash_completion
>  create mode 100644 src/cli_new/pip-requirements.txt
>  create mode 100644 src/cli_new/pylint.config
> ```

Presumably, that's because your source directory contains leftover 
directories/files (like `*.pyc` files, which prevent git from deleting the 
`cli_new` folder):
```
$ support/apply-reviews.py -n -r 50910 -c
2016-10-05 10:49:31 URL:https://reviews.apache.org/r/50907/diff/raw/ 
[16055/16055] -> "50907.patch" [1]
No C++ files to lint
[detached HEAD faed65f] Abstracted mesos-style.py to wrap the cpp linter in a 
class.
 Author: Kevin Klues 
 1 file changed, 249 insertions(+), 169 deletions(-)
 rewrite support/mesos-style.py (95%)
2016-10-05 10:49:32 URL:https://reviews.apache.org/r/50910/diff/raw/ 
[2997/2997] -> "50910.patch" [1]
No C++ files to lint
Could not find 'src/cli_new'
Please run from the root of the mesos source directory
```

If I do:
```
$ support/apply-reviews.py -n -r 50907
$ support/apply-reviews.py -n -r 50912 # <- reordered this one.
$ support/apply-reviews.py -n -r 50910
```

It applies cleanly.


- Joseph


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


On Aug. 11, 2016, 2:54 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 11, 2016, 2:54 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> 

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

> On Oct. 5, 2016, 12:18 a.m., Joseph Wu wrote:
> > support/mesos-style.py, lines 270-272
> > 
> >
> > It may help to change the order of reviews (no rebase necessary, just 
> > the "depends on" field).  
> > 
> > Because you add a linter in this review, that depends on the next one ( 
> > https://reviews.apache.org/r/50912/ ), this review fails to lint itself :)
> 
> Kevin Klues wrote:
> I'm confused. The order of these patches (and their dependencies) should 
> be correct. This patch "depends on" 50907, not 50912. Also, when I 
> cherry-pick each of these patches onto master individually, they all get 
> linted just as expected.

For example, from master:
```
klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50907
2016-10-04 20:31:30 URL:https://reviews.apache.org/r/50907/diff/raw/ 
[16055/16055] -> "50907.patch" [1]
No C++ files to lint
[cli-test d536361] Abstracted mesos-style.py to wrap the cpp linter in a class.
 1 file changed, 249 insertions(+), 169 deletions(-)
 rewrite support/mesos-style.py (95%)
klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50910
2016-10-04 20:31:39 URL:https://reviews.apache.org/r/50910/diff/raw/ 
[2997/2997] -> "50910.patch" [1]
No C++ files to lint
No Python files to lint
[cli-test 2b9502e] Added a python linter to mesos-style.cpp.
 Author: Haris Choudhary 
 1 file changed, 67 insertions(+), 2 deletions(-)
klueska@c99:~/projects/mesos$ support/apply-reviews.py -r 50912
2016-10-04 20:31:51 URL:https://reviews.apache.org/r/50912/diff/raw/ 
[40930/40930] -> "50912.patch" [1]
No C++ files to lint
Checking 8 Python files
Total errors found: 0
[cli-test 88e5726] Added the infrastructure for a new python-based CLI.
 20 files changed, 1019 insertions(+), 3 deletions(-)
 create mode 100644 src/cli_new/.gitignore
 create mode 100644 src/cli_new/README.md
 create mode 100644 src/cli_new/activate
 create mode 100644 src/cli_new/bin/config.py
 create mode 100644 src/cli_new/bin/main.py
 create mode 100755 src/cli_new/bin/mesos
 create mode 100755 src/cli_new/bootstrap
 create mode 100644 src/cli_new/deactivate
 create mode 100644 src/cli_new/lib/mesos/__init__.py
 create mode 100644 src/cli_new/lib/mesos/docopt.py
 create mode 100644 src/cli_new/lib/mesos/exceptions.py
 create mode 100644 src/cli_new/lib/mesos/plugins/__init__.py
 create mode 100644 src/cli_new/lib/mesos/plugins/base.py
 create mode 100644 src/cli_new/lib/mesos/util.py
 create mode 100644 src/cli_new/mesos.bash_completion
 create mode 100644 src/cli_new/pip-requirements.txt
 create mode 100644 src/cli_new/pylint.config
```


- Kevin


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


On Aug. 11, 2016, 9:54 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 11, 2016, 9:54 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

> On Oct. 5, 2016, 12:18 a.m., Joseph Wu wrote:
> > support/mesos-style.py, lines 270-272
> > 
> >
> > It may help to change the order of reviews (no rebase necessary, just 
> > the "depends on" field).  
> > 
> > Because you add a linter in this review, that depends on the next one ( 
> > https://reviews.apache.org/r/50912/ ), this review fails to lint itself :)

I'm confused. The order of these patches (and their dependencies) should be 
correct. This patch "depends on" 50907, not 50912. Also, when I cherry-pick 
each of these patches onto master individually, they all get linted just as 
expected.


- Kevin


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


On Aug. 11, 2016, 9:54 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 11, 2016, 9:54 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Joseph Wu]

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


Fix it, then Ship it!





support/mesos-style.py (lines 270 - 272)


It may help to change the order of reviews (no rebase necessary, just the 
"depends on" field).  

Because you add a linter in this review, that depends on the next one ( 
https://reviews.apache.org/r/50912/ ), this review fails to lint itself :)


- Joseph Wu


On Aug. 11, 2016, 2:54 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 11, 2016, 2:54 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Aug. 11, 2016, 9:54 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 11, 2016, 9:54 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50907, 50910]

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. 11, 2016, 9:54 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 11, 2016, 9:54 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Haris Choudhary]

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

(Updated Aug. 11, 2016, 9:54 p.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


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


Repository: mesos


Description
---

It currently doesn't run over any files in the code base, but we will
be adding the new python CLI in a subsequent commit, which will use
this new linter.


Diffs (updated)
-

  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---


Thanks,

Haris Choudhary

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

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




support/mesos-style.py (line 215)


Since this is an internal function, I would prefix it with "`__`".



support/mesos-style.py (line 217)


Do you really need this temporary variable if you just use it once. Why not 
do the `os.path.join()` directly in the `isdir()` call?



support/mesos-style.py (line 244)


You don't use this variable anywhere.



support/mesos-style.py (lines 250 - 251)


I would either call the format strings in here "virtualenv_dir", "lib_dir", 
and "bin_dir" or just "virtualenv", "bin", and "lib". Having a "p" at the end 
of the variable names is confusing and inconsistent (at least with the name 
"virtualenv").



support/mesos-style.py (line 251)


There seems to be an extra space here.



support/mesos-style.py (line 258)


It looks like there is 1 extra space here.


- Kevin Klues


On Aug. 11, 2016, 8:02 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 11, 2016, 8:02 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Haris Choudhary]

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

(Updated Aug. 11, 2016, 8:02 p.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


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


Repository: mesos


Description
---

It currently doesn't run over any files in the code base, but we will
be adding the new python CLI in a subsequent commit, which will use
this new linter.


Diffs (updated)
-

  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---


Thanks,

Haris Choudhary

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Joseph Wu]

> On Aug. 10, 2016, 12:02 p.m., Kevin Klues wrote:
> > support/mesos-style.py, lines 228-234
> > 
> >
> > I know we didn't talk about this, but I realized recently that we 
> > actually *have* to run pylint inside the virtual environment, otherwise it 
> > runs using the system python, which is not what we want. Especially for 
> > import libraries.
> 
> Haris Choudhary wrote:
> There are two approaches to this:
> 
> 1) We can either activate the virtualenv from the CLI and than run 
> pylint. But that means if the virtualenv is not created within the CLI we 
> will have to create it and activate it.
> 
> 2) We can integrate the virtualenv to the project wide bootstrap and thus 
> ensuring the the virtualenv is created for the project on bootstrapping 
> mesos. This seems to be the better way to do it however might require 
> significant changes as opposed to (1). It'd be desirable to have a 
> project-wide virtualenv at some point however even if we choose to not do so 
> right now. A thing to note here is that if we integrate it to the project 
> wide bootstrap, we'll need users to have virtualenv installed for mesos 
> instead of only the CLI.
> 
> Kevin Klues wrote:
> I'd prefer #2, but it's a bigger change and has the downside that the 
> virtualenv dependency will become immediate if we add it to the top-level 
> bootstrap. Curious what @vinodkone thinks.
> 
> Kevin Klues wrote:
> I think this should suffice for now (including fixes to the indentation):
> ```
> cli_dir = os.path.abspath(self.source_dirs[0])
> source_files = ' '.join(source_paths)
> 
> p = subprocess.Popen(
> ['source {virtualenv}/bin/activate; \
>  PYTHONPATH={lib_dir}:{bin_dir} pylint --rcfile={config} 
> --ignore={ignore} {files}'.\
>  format(virtualenv=os.path.join(cli_dir, '.virtualenv'),
> lib_dir=os.path.join(cli_dir, 'lib'),
> bin_dir=os.path.join(cli_dir, 'bin'),
> config=os.path.join(cli_dir, 'pylint.config'),
> ignore=os.path.join(cli_dir, 'bin', 'mesos'),
> files=source_files)],
> shell=True, stdout=subprocess.PIPE)
> ```
> 
> Kevin Klues wrote:
> Though,a ctually, this should come in a subsequent commit. Just fix the 
> indenting and the other comments from Vinod and I'll add this in as part of 
> my commit that actually populates the `cli_dir`.

I would prefer something like this:

```
p = subprocess.Popen([
'pylint', 
'--rcfile=%s' % os.path.join(cli_dir, 'pylint.config'),
'--ignore=%s' % os.path.join(cli_dir, 'bin', 'mesos'),
] + source_paths,
env={ "PYTHONPATH" : "%s:%s" % (lib_path, bin_path)},
stdout=subprocess.PIPE)
```

No `shell=True`.  As for how to activate a virtualenv... I'm not sure.  But it 
should still be possible without using `shell=True`, I hope.


- Joseph


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


On Aug. 10, 2016, 3:43 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 10, 2016, 3:43 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

> On Aug. 10, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/mesos-style.py, lines 228-234
> > 
> >
> > I know we didn't talk about this, but I realized recently that we 
> > actually *have* to run pylint inside the virtual environment, otherwise it 
> > runs using the system python, which is not what we want. Especially for 
> > import libraries.
> 
> Haris Choudhary wrote:
> There are two approaches to this:
> 
> 1) We can either activate the virtualenv from the CLI and than run 
> pylint. But that means if the virtualenv is not created within the CLI we 
> will have to create it and activate it.
> 
> 2) We can integrate the virtualenv to the project wide bootstrap and thus 
> ensuring the the virtualenv is created for the project on bootstrapping 
> mesos. This seems to be the better way to do it however might require 
> significant changes as opposed to (1). It'd be desirable to have a 
> project-wide virtualenv at some point however even if we choose to not do so 
> right now. A thing to note here is that if we integrate it to the project 
> wide bootstrap, we'll need users to have virtualenv installed for mesos 
> instead of only the CLI.
> 
> Kevin Klues wrote:
> I'd prefer #2, but it's a bigger change and has the downside that the 
> virtualenv dependency will become immediate if we add it to the top-level 
> bootstrap. Curious what @vinodkone thinks.
> 
> Kevin Klues wrote:
> I think this should suffice for now (including fixes to the indentation):
> ```
> cli_dir = os.path.abspath(self.source_dirs[0])
> source_files = ' '.join(source_paths)
> 
> p = subprocess.Popen(
> ['source {virtualenv}/bin/activate; \
>  PYTHONPATH={lib_dir}:{bin_dir} pylint --rcfile={config} 
> --ignore={ignore} {files}'.\
>  format(virtualenv=os.path.join(cli_dir, '.virtualenv'),
> lib_dir=os.path.join(cli_dir, 'lib'),
> bin_dir=os.path.join(cli_dir, 'bin'),
> config=os.path.join(cli_dir, 'pylint.config'),
> ignore=os.path.join(cli_dir, 'bin', 'mesos'),
> files=source_files)],
> shell=True, stdout=subprocess.PIPE)
> ```

Though,a ctually, this should come in a subsequent commit. Just fix the 
indenting and the other comments from Vinod and I'll add this in as part of my 
commit that actually populates the `cli_dir`.


- Kevin


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


On Aug. 10, 2016, 10:43 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 10, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

> On Aug. 10, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/mesos-style.py, lines 228-234
> > 
> >
> > I know we didn't talk about this, but I realized recently that we 
> > actually *have* to run pylint inside the virtual environment, otherwise it 
> > runs using the system python, which is not what we want. Especially for 
> > import libraries.
> 
> Haris Choudhary wrote:
> There are two approaches to this:
> 
> 1) We can either activate the virtualenv from the CLI and than run 
> pylint. But that means if the virtualenv is not created within the CLI we 
> will have to create it and activate it.
> 
> 2) We can integrate the virtualenv to the project wide bootstrap and thus 
> ensuring the the virtualenv is created for the project on bootstrapping 
> mesos. This seems to be the better way to do it however might require 
> significant changes as opposed to (1). It'd be desirable to have a 
> project-wide virtualenv at some point however even if we choose to not do so 
> right now. A thing to note here is that if we integrate it to the project 
> wide bootstrap, we'll need users to have virtualenv installed for mesos 
> instead of only the CLI.
> 
> Kevin Klues wrote:
> I'd prefer #2, but it's a bigger change and has the downside that the 
> virtualenv dependency will become immediate if we add it to the top-level 
> bootstrap. Curious what @vinodkone thinks.

I think this should suffice for now (including fixes to the indentation):
```
cli_dir = os.path.abspath(self.source_dirs[0])
source_files = ' '.join(source_paths)

p = subprocess.Popen(
['source {virtualenv}/bin/activate; \
 PYTHONPATH={lib_dir}:{bin_dir} pylint --rcfile={config} 
--ignore={ignore} {files}'.\
 format(virtualenv=os.path.join(cli_dir, '.virtualenv'),
lib_dir=os.path.join(cli_dir, 'lib'),
bin_dir=os.path.join(cli_dir, 'bin'),
config=os.path.join(cli_dir, 'pylint.config'),
ignore=os.path.join(cli_dir, 'bin', 'mesos'),
files=source_files)],
shell=True, stdout=subprocess.PIPE)
```


- Kevin


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


On Aug. 10, 2016, 10:43 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 10, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

> On Aug. 10, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/mesos-style.py, lines 228-234
> > 
> >
> > I know we didn't talk about this, but I realized recently that we 
> > actually *have* to run pylint inside the virtual environment, otherwise it 
> > runs using the system python, which is not what we want. Especially for 
> > import libraries.
> 
> Haris Choudhary wrote:
> There are two approaches to this:
> 
> 1) We can either activate the virtualenv from the CLI and than run 
> pylint. But that means if the virtualenv is not created within the CLI we 
> will have to create it and activate it.
> 
> 2) We can integrate the virtualenv to the project wide bootstrap and thus 
> ensuring the the virtualenv is created for the project on bootstrapping 
> mesos. This seems to be the better way to do it however might require 
> significant changes as opposed to (1). It'd be desirable to have a 
> project-wide virtualenv at some point however even if we choose to not do so 
> right now. A thing to note here is that if we integrate it to the project 
> wide bootstrap, we'll need users to have virtualenv installed for mesos 
> instead of only the CLI.

I'd prefer #2, but it's a bigger change and has the downside that the 
virtualenv dependency will become immediate if we add it to the top-level 
bootstrap. Curious what @vinodkone thinks.


- Kevin


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


On Aug. 10, 2016, 10:43 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 10, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Haris Choudhary]

> On Aug. 10, 2016, 4:53 a.m., Vinod Kone wrote:
> > support/mesos-style.py, line 242
> > 
> >
> > Don't follow this comment?

I meant that as a note to myself. Forgot to remove it. I was talking about how 
pylint does not give us access to the number of errors and so we need to parse 
the output to obtain our result.


- Haris


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


On Aug. 10, 2016, 10:43 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 10, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Haris Choudhary]

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

(Updated Aug. 10, 2016, 10:43 p.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


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


Repository: mesos


Description
---

It currently doesn't run over any files in the code base, but we will
be adding the new python CLI in a subsequent commit, which will use
this new linter.


Diffs (updated)
-

  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---


Thanks,

Haris Choudhary

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Haris Choudhary]

> On Aug. 10, 2016, 4:53 a.m., Vinod Kone wrote:
> > support/mesos-style.py, lines 228-234
> > 
> >
> > this indentation looks weird? or is it just RB?

I fixed that. Hope its better now.


- Haris


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


On Aug. 10, 2016, 10:43 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 10, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Haris Choudhary]

> On Aug. 10, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/mesos-style.py, lines 228-234
> > 
> >
> > I know we didn't talk about this, but I realized recently that we 
> > actually *have* to run pylint inside the virtual environment, otherwise it 
> > runs using the system python, which is not what we want. Especially for 
> > import libraries.

There are two approaches to this:

1) We can either activate the virtualenv from the CLI and than run pylint. But 
that means if the virtualenv is not created within the CLI we will have to 
create it and activate it.

2) We can integrate the virtualenv to the project wide bootstrap and thus 
ensuring the the virtualenv is created for the project on bootstrapping mesos. 
This seems to be the better way to do it however might require significant 
changes as opposed to (1). It'd be desirable to have a project-wide virtualenv 
at some point however even if we choose to not do so right now. A thing to note 
here is that if we integrate it to the project wide bootstrap, we'll need users 
to have virtualenv installed for mesos instead of only the CLI.


- Haris


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


On Aug. 8, 2016, 10:10 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 8, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

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




support/mesos-style.py (lines 228 - 234)


I know we didn't talk about this, but I realized recently that we actually 
*have* to run pylint inside the virtual environment, otherwise it runs using 
the system python, which is not what we want. Especially for import 
libraries.


- Kevin Klues


On Aug. 8, 2016, 10:10 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 8, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Vinod Kone]

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




support/mesos-style.py (line 219)


is this the right URL?



support/mesos-style.py (line 227)


s/pylint_proc/p/ to be consistent with CppLinter()



support/mesos-style.py (lines 228 - 234)


this indentation looks weird? or is it just RB?



support/mesos-style.py (line 238)


this looks different than how we extracted stderr in CppLinter(); can we 
make it consistent?



support/mesos-style.py (line 242)


Don't follow this comment?


- Vinod Kone


On Aug. 8, 2016, 10:10 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 8, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50907, 50910]

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, 10:10 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 8, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Review Request 50910: Added a python linter to mesos-style.cpp.

[Haris Choudhary]

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

Review request for mesos, Kevin Klues and Vinod Kone.


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


Repository: mesos


Description
---

It currently doesn't run over any files in the code base, but we will
be adding the new python CLI in a subsequent commit, which will use
this new linter.


Diffs
-

  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---


Thanks,

Haris Choudhary