> On Oct. 5, 2016, 12:18 a.m., Joseph Wu wrote: > > support/mesos-style.py, lines 270-272 > > <https://reviews.apache.org/r/50910/diff/4/?file=1471508#file1471508line270> > > > > 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 <haris0...@gmail.com> > 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 <klue...@gmail.com> > 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 > 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 > >
