> On Oct. 4, 2016, 5:18 p.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 <[email protected]> > 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 <[email protected]> > 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: > 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 > >
