You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2014/10/08 01:18:54 UTC
Review Request 26428: Fixing python style violations.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26428/
-----------------------------------------------------------
Review request for Aurora and Brian Wickman.
Repository: aurora
Description
-------
Pre-commit hook was no longer working properly. Added target to match jenkins definition.
Diffs
-----
build-support/hooks/pre-commit 24646e7b1c43a0177beb0084dd2af4c4b3dc686c
src/test/python/apache/aurora/client/api/test_updater.py fc6a057c6c650d9ac9800b009e544dfad0c809bf
src/test/python/apache/aurora/client/cli/test_status.py 38ffdb86c5f577ebf3a482128588331a63af15d1
src/test/python/apache/aurora/client/cli/util.py ff7eda20dbba073c8b24fbe3f4389292aab2d128
Diff: https://reviews.apache.org/r/26428/diff/
Testing
-------
./build-support/hooks/pre-commit
Performing Python import order check.
SUCCESS
Performing Python checkstyle.
SUCCESS
$ build-support/python/checkstyle-check src
Thanks,
Maxim Khutornenko
Re: Review Request 26428: Fixing python style violations.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26428/
-----------------------------------------------------------
(Updated Oct. 8, 2014, 7:22 p.m.)
Review request for Aurora and Brian Wickman.
Changes
-------
Fixing more violations.
Repository: aurora
Description
-------
Pre-commit hook was no longer working properly. Added target to match jenkins definition.
Diffs (updated)
-----
build-support/hooks/pre-commit 24646e7b1c43a0177beb0084dd2af4c4b3dc686c
src/main/python/apache/aurora/executor/gc_executor.py a11feb910cd116f53c9949343dfe7ecbd17d793c
src/test/python/apache/aurora/client/api/test_updater.py fc6a057c6c650d9ac9800b009e544dfad0c809bf
src/test/python/apache/aurora/client/cli/test_create.py 88d1b350b09012684bb4f7263657cdff083df215
src/test/python/apache/aurora/client/cli/test_inspect.py b64fbf9864693d8758baf01ba2293df525adebb7
src/test/python/apache/aurora/client/cli/test_status.py 38ffdb86c5f577ebf3a482128588331a63af15d1
src/test/python/apache/aurora/client/cli/util.py 4e9b6362728ecfd6e8a6efbc433f63f42aef60ad
Diff: https://reviews.apache.org/r/26428/diff/
Testing
-------
./build-support/hooks/pre-commit
Performing Python import order check.
SUCCESS
Performing Python checkstyle.
SUCCESS
$ build-support/python/checkstyle-check src
Thanks,
Maxim Khutornenko
Re: Review Request 26428: Fixing python style violations.
Posted by Brian Wickman <wi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26428/#review55857
-----------------------------------------------------------
Ship it!
Ship It!
- Brian Wickman
On Oct. 8, 2014, 12:17 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26428/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2014, 12:17 a.m.)
>
>
> Review request for Aurora and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Pre-commit hook was no longer working properly. Added target to match jenkins definition.
>
>
> Diffs
> -----
>
> build-support/hooks/pre-commit 24646e7b1c43a0177beb0084dd2af4c4b3dc686c
> src/test/python/apache/aurora/client/api/test_updater.py fc6a057c6c650d9ac9800b009e544dfad0c809bf
> src/test/python/apache/aurora/client/cli/test_status.py 38ffdb86c5f577ebf3a482128588331a63af15d1
> src/test/python/apache/aurora/client/cli/util.py ff7eda20dbba073c8b24fbe3f4389292aab2d128
>
> Diff: https://reviews.apache.org/r/26428/diff/
>
>
> Testing
> -------
>
> ./build-support/hooks/pre-commit
> Performing Python import order check.
> SUCCESS
> Performing Python checkstyle.
> SUCCESS
>
> $ build-support/python/checkstyle-check src
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 26428: Fixing python style violations.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26428/
-----------------------------------------------------------
(Updated Oct. 8, 2014, 12:17 a.m.)
Review request for Aurora and Brian Wickman.
Changes
-------
CR comments.
Repository: aurora
Description
-------
Pre-commit hook was no longer working properly. Added target to match jenkins definition.
Diffs (updated)
-----
build-support/hooks/pre-commit 24646e7b1c43a0177beb0084dd2af4c4b3dc686c
src/test/python/apache/aurora/client/api/test_updater.py fc6a057c6c650d9ac9800b009e544dfad0c809bf
src/test/python/apache/aurora/client/cli/test_status.py 38ffdb86c5f577ebf3a482128588331a63af15d1
src/test/python/apache/aurora/client/cli/util.py ff7eda20dbba073c8b24fbe3f4389292aab2d128
Diff: https://reviews.apache.org/r/26428/diff/
Testing
-------
./build-support/hooks/pre-commit
Performing Python import order check.
SUCCESS
Performing Python checkstyle.
SUCCESS
$ build-support/python/checkstyle-check src
Thanks,
Maxim Khutornenko
Re: Review Request 26428: Fixing python style violations.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Oct. 7, 2014, 11:31 p.m., Brian Wickman wrote:
> > build-support/hooks/pre-commit, line 41
> > <https://reviews.apache.org/r/26428/diff/1/?file=714987#file714987line41>
> >
> > the reason i didn't add this to the pre-commit is beause the default behavior for the pre-commit hook is to only check the diff against master.
> > it's possible that it needs more context e.g. +-2 lines in the file beyond the diff, so it might miss stuff.
> >
> > doing against source for every commit might be too time consuming. out of curiosity, how long does it take to run this check on your laptop?
It's between 15-20 seconds now, which is much slower than usual but I'd rather make it slower and useful or drop completely and rely on jenkins script. Your call.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26428/#review55729
-----------------------------------------------------------
On Oct. 7, 2014, 11:18 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26428/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2014, 11:18 p.m.)
>
>
> Review request for Aurora and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Pre-commit hook was no longer working properly. Added target to match jenkins definition.
>
>
> Diffs
> -----
>
> build-support/hooks/pre-commit 24646e7b1c43a0177beb0084dd2af4c4b3dc686c
> src/test/python/apache/aurora/client/api/test_updater.py fc6a057c6c650d9ac9800b009e544dfad0c809bf
> src/test/python/apache/aurora/client/cli/test_status.py 38ffdb86c5f577ebf3a482128588331a63af15d1
> src/test/python/apache/aurora/client/cli/util.py ff7eda20dbba073c8b24fbe3f4389292aab2d128
>
> Diff: https://reviews.apache.org/r/26428/diff/
>
>
> Testing
> -------
>
> ./build-support/hooks/pre-commit
> Performing Python import order check.
> SUCCESS
> Performing Python checkstyle.
> SUCCESS
>
> $ build-support/python/checkstyle-check src
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 26428: Fixing python style violations.
Posted by Brian Wickman <wi...@apache.org>.
> On Oct. 7, 2014, 11:31 p.m., Brian Wickman wrote:
> > build-support/hooks/pre-commit, line 41
> > <https://reviews.apache.org/r/26428/diff/1/?file=714987#file714987line41>
> >
> > the reason i didn't add this to the pre-commit is beause the default behavior for the pre-commit hook is to only check the diff against master.
> > it's possible that it needs more context e.g. +-2 lines in the file beyond the diff, so it might miss stuff.
> >
> > doing against source for every commit might be too time consuming. out of curiosity, how long does it take to run this check on your laptop?
>
> Maxim Khutornenko wrote:
> It's between 15-20 seconds now, which is much slower than usual but I'd rather make it slower and useful or drop completely and rely on jenkins script. Your call.
We should either fix --diff or change its behavior to be more conservative by performing checkstyle on the entire file if it is involved in the diff. In the meantime, I will give a ship, since pre-commits are still opt-in.
- Brian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26428/#review55729
-----------------------------------------------------------
On Oct. 8, 2014, 12:17 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26428/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2014, 12:17 a.m.)
>
>
> Review request for Aurora and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Pre-commit hook was no longer working properly. Added target to match jenkins definition.
>
>
> Diffs
> -----
>
> build-support/hooks/pre-commit 24646e7b1c43a0177beb0084dd2af4c4b3dc686c
> src/test/python/apache/aurora/client/api/test_updater.py fc6a057c6c650d9ac9800b009e544dfad0c809bf
> src/test/python/apache/aurora/client/cli/test_status.py 38ffdb86c5f577ebf3a482128588331a63af15d1
> src/test/python/apache/aurora/client/cli/util.py ff7eda20dbba073c8b24fbe3f4389292aab2d128
>
> Diff: https://reviews.apache.org/r/26428/diff/
>
>
> Testing
> -------
>
> ./build-support/hooks/pre-commit
> Performing Python import order check.
> SUCCESS
> Performing Python checkstyle.
> SUCCESS
>
> $ build-support/python/checkstyle-check src
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 26428: Fixing python style violations.
Posted by Brian Wickman <wi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26428/#review55729
-----------------------------------------------------------
build-support/hooks/pre-commit
<https://reviews.apache.org/r/26428/#comment96117>
this argument is ignored for isort-check/isort-run
build-support/hooks/pre-commit
<https://reviews.apache.org/r/26428/#comment96118>
the reason i didn't add this to the pre-commit is beause the default behavior for the pre-commit hook is to only check the diff against master.
it's possible that it needs more context e.g. +-2 lines in the file beyond the diff, so it might miss stuff.
doing against source for every commit might be too time consuming. out of curiosity, how long does it take to run this check on your laptop?
- Brian Wickman
On Oct. 7, 2014, 11:18 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26428/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2014, 11:18 p.m.)
>
>
> Review request for Aurora and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Pre-commit hook was no longer working properly. Added target to match jenkins definition.
>
>
> Diffs
> -----
>
> build-support/hooks/pre-commit 24646e7b1c43a0177beb0084dd2af4c4b3dc686c
> src/test/python/apache/aurora/client/api/test_updater.py fc6a057c6c650d9ac9800b009e544dfad0c809bf
> src/test/python/apache/aurora/client/cli/test_status.py 38ffdb86c5f577ebf3a482128588331a63af15d1
> src/test/python/apache/aurora/client/cli/util.py ff7eda20dbba073c8b24fbe3f4389292aab2d128
>
> Diff: https://reviews.apache.org/r/26428/diff/
>
>
> Testing
> -------
>
> ./build-support/hooks/pre-commit
> Performing Python import order check.
> SUCCESS
> Performing Python checkstyle.
> SUCCESS
>
> $ build-support/python/checkstyle-check src
>
>
> Thanks,
>
> Maxim Khutornenko
>
>