You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <bb...@apache.org> on 2019/07/30 21:01:27 UTC
Review Request 71205: Switch commit hooks to pre-commit.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/
-----------------------------------------------------------
Review request for mesos and Till Toenshoff.
Bugs: MESOS-9360
https://issues.apache.org/jira/browse/MESOS-9360
Repository: mesos
Description
-------
This patch switches commit hooks to be orchestrated by the pre-commit
tool mirroring the previous linters invoked through git commit
hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
Using pre-commit removes the burden of maintaining
`support/mesos-style.py`, making sure that hooks have the expected
environment (e.g., Python version, Node installed). Additionally,
upstream provides a number of additional linters which are not hard to
add to Mesos' hooks.
Diffs
-----
.pre-commit-config.yaml PRE-CREATION
bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c
Diff: https://reviews.apache.org/r/71205/diff/1/
Testing
-------
* used successfully for a couple of months
Thanks,
Benjamin Bannier
Re: Review Request 71205: Switch commit hooks to pre-commit.
Posted by Benjamin Bannier <bb...@apache.org>.
> On Aug. 14, 2019, 1:04 p.m., Benno Evers wrote:
> > bootstrap
> > Line 55 (original), 55 (patched)
> > <https://reviews.apache.org/r/71205/diff/1/?file=2158248#file2158248line55>
> >
> > Copying my comment from slack here so the discussion isn't split over too many places:
> >
> > As we discussed privately yesterday, I think installing this in bootstrap is a bit problematic because that is also part of the source tarball, used by non-developers to build mesos w/o even having a git repository. Additionally, I think I'd be not particularly amused if I cloned a random open-source project and the first thing it does is install a bunch of stuff in my home directory.
> >
> > I'm not sure if it's possible to implement, but imho the ideal workflow would be something like this:
> >
> > ```
> > $ git commit -m "Foo the bar."
> > WARNING: Your commit touched a `.cpp` file, but `cpplint` is not installed on your system. It is highly recommended to install it to avoid embarrassing mistakes.
> >
> > You can also run `pre-commit ????` to automatically install a usable version of `cpplint` in you home directory.
> > ```
Totally agree on the mix of building and dev setup in `bootstrap` is problematic. Let me break out an `autogen.sh` to be used for setting up a build env, and `boostrap` also setting up a dev env.
We already download packages from the internet from `mesos-style.py` when e.g., setting up the virtualenv to run node linters. There is however no caching so one needs to potentially download this again and again after clearing the build dir; with `pre-commit` files are cached in `$HOME/.cache` and even for different linter versions or configs with the lifetime managed explicitly with `pre-commit`. I feel this is a superior, more controllable approach than pulling stuff with weird heuristics (`mesos-style.py` hardcodes a few of these and they tend to perform more work than necessary).
The only dependency of the linters now becomes `pre-commit` (which needs to be clearly documented) and potentially some Python interpreters or sim. Linters like e.g., eslint are automatically set up and do not need to be installed by users (in fact we want to control the exact versions of these tools, so `pre-commit` enforcing that is great). `cpplint` is a linter present in the source tree and does not need to be installed.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/#review217200
-----------------------------------------------------------
On July 30, 2019, 11:01 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> -----------------------------------------------------------
>
> (Updated July 30, 2019, 11:01 p.m.)
>
>
> Review request for mesos and Till Toenshoff.
>
>
> Bugs: MESOS-9360
> https://issues.apache.org/jira/browse/MESOS-9360
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
>
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
>
>
> Diffs
> -----
>
> .pre-commit-config.yaml PRE-CREATION
> bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c
>
>
> Diff: https://reviews.apache.org/r/71205/diff/1/
>
>
> Testing
> -------
>
> * used successfully for a couple of months
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71205: Switch commit hooks to pre-commit.
Posted by Benjamin Bannier <bb...@apache.org>.
> On Aug. 14, 2019, 1:04 p.m., Benno Evers wrote:
> > bootstrap
> > Line 55 (original), 55 (patched)
> > <https://reviews.apache.org/r/71205/diff/1/?file=2158248#file2158248line55>
> >
> > Copying my comment from slack here so the discussion isn't split over too many places:
> >
> > As we discussed privately yesterday, I think installing this in bootstrap is a bit problematic because that is also part of the source tarball, used by non-developers to build mesos w/o even having a git repository. Additionally, I think I'd be not particularly amused if I cloned a random open-source project and the first thing it does is install a bunch of stuff in my home directory.
> >
> > I'm not sure if it's possible to implement, but imho the ideal workflow would be something like this:
> >
> > ```
> > $ git commit -m "Foo the bar."
> > WARNING: Your commit touched a `.cpp` file, but `cpplint` is not installed on your system. It is highly recommended to install it to avoid embarrassing mistakes.
> >
> > You can also run `pre-commit ????` to automatically install a usable version of `cpplint` in you home directory.
> > ```
>
> Benjamin Bannier wrote:
> Totally agree on the mix of building and dev setup in `bootstrap` is problematic. Let me break out an `autogen.sh` to be used for setting up a build env, and `boostrap` also setting up a dev env.
>
> We already download packages from the internet from `mesos-style.py` when e.g., setting up the virtualenv to run node linters. There is however no caching so one needs to potentially download this again and again after clearing the build dir; with `pre-commit` files are cached in `$HOME/.cache` and even for different linter versions or configs with the lifetime managed explicitly with `pre-commit`. I feel this is a superior, more controllable approach than pulling stuff with weird heuristics (`mesos-style.py` hardcodes a few of these and they tend to perform more work than necessary).
>
> The only dependency of the linters now becomes `pre-commit` (which needs to be clearly documented) and potentially some Python interpreters or sim. Linters like e.g., eslint are automatically set up and do not need to be installed by users (in fact we want to control the exact versions of these tools, so `pre-commit` enforcing that is great). `cpplint` is a linter present in the source tree and does not need to be installed.
>
> Benjamin Bannier wrote:
> I added a dummy script for `support/mesos-style.py` to ease the transition. In another patch I'll also split installation of dev tools out of `./bootstrap` to reduce dependencies non-contributors need to install, and add additional documentation.
Broke installation of dev tools out into https://reviews.apache.org/r/71299/; transitional `./support/mesos-style.py` is removed with https://reviews.apache.org/r/71299/.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/#review217200
-----------------------------------------------------------
On July 30, 2019, 11:01 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> -----------------------------------------------------------
>
> (Updated July 30, 2019, 11:01 p.m.)
>
>
> Review request for mesos and Till Toenshoff.
>
>
> Bugs: MESOS-9360
> https://issues.apache.org/jira/browse/MESOS-9360
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
>
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
>
>
> Diffs
> -----
>
> .pre-commit-config.yaml PRE-CREATION
> bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c
>
>
> Diff: https://reviews.apache.org/r/71205/diff/2/
>
>
> Testing
> -------
>
> * used successfully for a couple of months
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71205: Switch commit hooks to pre-commit.
Posted by Benjamin Bannier <bb...@apache.org>.
> On Aug. 14, 2019, 1:04 p.m., Benno Evers wrote:
> > bootstrap
> > Line 55 (original), 55 (patched)
> > <https://reviews.apache.org/r/71205/diff/1/?file=2158248#file2158248line55>
> >
> > Copying my comment from slack here so the discussion isn't split over too many places:
> >
> > As we discussed privately yesterday, I think installing this in bootstrap is a bit problematic because that is also part of the source tarball, used by non-developers to build mesos w/o even having a git repository. Additionally, I think I'd be not particularly amused if I cloned a random open-source project and the first thing it does is install a bunch of stuff in my home directory.
> >
> > I'm not sure if it's possible to implement, but imho the ideal workflow would be something like this:
> >
> > ```
> > $ git commit -m "Foo the bar."
> > WARNING: Your commit touched a `.cpp` file, but `cpplint` is not installed on your system. It is highly recommended to install it to avoid embarrassing mistakes.
> >
> > You can also run `pre-commit ????` to automatically install a usable version of `cpplint` in you home directory.
> > ```
>
> Benjamin Bannier wrote:
> Totally agree on the mix of building and dev setup in `bootstrap` is problematic. Let me break out an `autogen.sh` to be used for setting up a build env, and `boostrap` also setting up a dev env.
>
> We already download packages from the internet from `mesos-style.py` when e.g., setting up the virtualenv to run node linters. There is however no caching so one needs to potentially download this again and again after clearing the build dir; with `pre-commit` files are cached in `$HOME/.cache` and even for different linter versions or configs with the lifetime managed explicitly with `pre-commit`. I feel this is a superior, more controllable approach than pulling stuff with weird heuristics (`mesos-style.py` hardcodes a few of these and they tend to perform more work than necessary).
>
> The only dependency of the linters now becomes `pre-commit` (which needs to be clearly documented) and potentially some Python interpreters or sim. Linters like e.g., eslint are automatically set up and do not need to be installed by users (in fact we want to control the exact versions of these tools, so `pre-commit` enforcing that is great). `cpplint` is a linter present in the source tree and does not need to be installed.
I added a dummy script for `support/mesos-style.py` to ease the transition. In another patch I'll also split installation of dev tools out of `./bootstrap` to reduce dependencies non-contributors need to install, and add additional documentation.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/#review217200
-----------------------------------------------------------
On July 30, 2019, 11:01 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> -----------------------------------------------------------
>
> (Updated July 30, 2019, 11:01 p.m.)
>
>
> Review request for mesos and Till Toenshoff.
>
>
> Bugs: MESOS-9360
> https://issues.apache.org/jira/browse/MESOS-9360
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
>
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
>
>
> Diffs
> -----
>
> .pre-commit-config.yaml PRE-CREATION
> bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c
>
>
> Diff: https://reviews.apache.org/r/71205/diff/2/
>
>
> Testing
> -------
>
> * used successfully for a couple of months
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71205: Switch commit hooks to pre-commit.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/#review217200
-----------------------------------------------------------
bootstrap
Line 55 (original), 55 (patched)
<https://reviews.apache.org/r/71205/#comment304493>
Copying my comment from slack here so the discussion isn't split over too many places:
As we discussed privately yesterday, I think installing this in bootstrap is a bit problematic because that is also part of the source tarball, used by non-developers to build mesos w/o even having a git repository. Additionally, I think I'd be not particularly amused if I cloned a random open-source project and the first thing it does is install a bunch of stuff in my home directory.
I'm not sure if it's possible to implement, but imho the ideal workflow would be something like this:
```
$ git commit -m "Foo the bar."
WARNING: Your commit touched a `.cpp` file, but `cpplint` is not installed on your system. It is highly recommended to install it to avoid embarrassing mistakes.
You can also run `pre-commit ????` to automatically install a usable version of `cpplint` in you home directory.
```
- Benno Evers
On July 30, 2019, 9:01 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> -----------------------------------------------------------
>
> (Updated July 30, 2019, 9:01 p.m.)
>
>
> Review request for mesos and Till Toenshoff.
>
>
> Bugs: MESOS-9360
> https://issues.apache.org/jira/browse/MESOS-9360
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
>
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
>
>
> Diffs
> -----
>
> .pre-commit-config.yaml PRE-CREATION
> bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c
>
>
> Diff: https://reviews.apache.org/r/71205/diff/1/
>
>
> Testing
> -------
>
> * used successfully for a couple of months
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71205: Switched commit hooks to pre-commit.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/#review217266
-----------------------------------------------------------
Ship it!
The pre-commit install was a bit non-obvious to me, I needed to do `sudo python3 -m pip install pre-commit`. that's more a `pre-commit` docs issue however.
- James Peach
On July 30, 2019, 9:01 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> -----------------------------------------------------------
>
> (Updated July 30, 2019, 9:01 p.m.)
>
>
> Review request for mesos and Till Toenshoff.
>
>
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
>
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
>
>
> Diffs
> -----
>
> .pre-commit-config.yaml PRE-CREATION
> docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c
> support/setup_dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4
> support/setup_dev.sh PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/71205/diff/6/
>
>
> Testing
> -------
>
> * used successfully for a couple of months
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71205: Switched commit hooks to pre-commit.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/#review217283
-----------------------------------------------------------
Ship it!
Ship It!
- Benno Evers
On Aug. 19, 2019, 7:20 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> -----------------------------------------------------------
>
> (Updated Aug. 19, 2019, 7:20 a.m.)
>
>
> Review request for mesos and Till Toenshoff.
>
>
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
>
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
>
>
> Diffs
> -----
>
> docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c
> support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b
> support/pre-commit-config.yaml PRE-CREATION
> support/setup-dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4
> support/setup-dev.sh PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/71205/diff/7/
>
>
> Testing
> -------
>
> * used successfully for a couple of months
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71205: Switched commit hooks to pre-commit.
Posted by Benjamin Bannier <bb...@apache.org>.
> On Aug. 26, 2019, 8:51 p.m., Till Toenshoff wrote:
> > When running the script, I noticed this
> > ```
> > [INFO] Installing environment for local.
> > [INFO] Once installed this environment will be reused.
> > [INFO] This may take a few minutes...
> > [INFO] Installing environment for local.
> > [INFO] Once installed this environment will be reused.
> > [INFO] This may take a few minutes...
> > ```
> > Why do we seemingly repeat that step? After this all further steps were non duplicated.
This is triggered by `pre-commit install-hooks` and seems to be a quirk of how `local` environments are set up. I am not sure there is anything we could or even should do about it.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/#review217439
-----------------------------------------------------------
On Aug. 27, 2019, 10:08 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2019, 10:08 a.m.)
>
>
> Review request for mesos and Till Toenshoff.
>
>
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
>
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
>
>
> Diffs
> -----
>
> docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c
> support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b
> support/pre-commit-config.yaml PRE-CREATION
> support/setup-dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4
> support/setup-dev.sh PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/71205/diff/9/
>
>
> Testing
> -------
>
> * used successfully for a couple of months
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71205: Switched commit hooks to pre-commit.
Posted by Till Toenshoff via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/#review217439
-----------------------------------------------------------
Fix it, then Ship it!
When running the script, I noticed this
```
[INFO] Installing environment for local.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for local.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
```
Why do we seemingly repeat that step? After this all further steps were non duplicated.
docs/advanced-contribution.md
Line 67 (original), 68 (patched)
<https://reviews.apache.org/r/71205/#comment304711>
"The hooks are set up with `./support/setup_dev.sh`"
We have to invoke it, so it does not really do much automatically, I'ld argue.
support/setup-dev.sh
Lines 57 (patched)
<https://reviews.apache.org/r/71205/#comment304712>
We generally strive for capitalizing product names - so let's have it this `Mesos` way everywhere please :)
support/setup-dev.sh
Lines 62 (patched)
<https://reviews.apache.org/r/71205/#comment304710>
Shouldnt there be an `exit(1)` here so we spare our users three more error messages?
- Till Toenshoff
On Aug. 20, 2019, 11:48 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> -----------------------------------------------------------
>
> (Updated Aug. 20, 2019, 11:48 a.m.)
>
>
> Review request for mesos and Till Toenshoff.
>
>
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
>
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
>
>
> Diffs
> -----
>
> docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c
> support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b
> support/pre-commit-config.yaml PRE-CREATION
> support/setup-dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4
> support/setup-dev.sh PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/71205/diff/8/
>
>
> Testing
> -------
>
> * used successfully for a couple of months
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71205: Switched commit hooks to pre-commit.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/
-----------------------------------------------------------
(Updated Sept. 10, 2019, 12:02 p.m.)
Review request for mesos and Till Toenshoff.
Changes
-------
Do not run pylint on CLI-related files
These files require certain packages to be installed. Previously mesos-style solved this by by executing special scripts which set up the virtual envs in which pylint could be run. Since pylint is also run as part of the test suite of these scripts we now remove the external linting here since it the effort exceeds the benefit.
Bugs: MESOS-9630
https://issues.apache.org/jira/browse/MESOS-9630
Repository: mesos
Description
-------
This patch switches commit hooks to be orchestrated by the pre-commit
tool mirroring the previous linters invoked through git commit
hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
Using pre-commit removes the burden of maintaining
`support/mesos-style.py`, making sure that hooks have the expected
environment (e.g., Python version, Node installed). Additionally,
upstream provides a number of additional linters which are not hard to
add to Mesos' hooks.
Diffs (updated)
-----
docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c
support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b
support/pre-commit-config.yaml PRE-CREATION
support/setup-dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4
support/setup-dev.sh PRE-CREATION
Diff: https://reviews.apache.org/r/71205/diff/12/
Changes: https://reviews.apache.org/r/71205/diff/11-12/
Testing
-------
* used successfully for a couple of months
Thanks,
Benjamin Bannier
Re: Review Request 71205: Switched commit hooks to pre-commit.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/
-----------------------------------------------------------
(Updated Aug. 28, 2019, 11:04 a.m.)
Review request for mesos and Till Toenshoff.
Bugs: MESOS-9630
https://issues.apache.org/jira/browse/MESOS-9630
Repository: mesos
Description
-------
This patch switches commit hooks to be orchestrated by the pre-commit
tool mirroring the previous linters invoked through git commit
hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
Using pre-commit removes the burden of maintaining
`support/mesos-style.py`, making sure that hooks have the expected
environment (e.g., Python version, Node installed). Additionally,
upstream provides a number of additional linters which are not hard to
add to Mesos' hooks.
Diffs (updated)
-----
docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c
support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b
support/pre-commit-config.yaml PRE-CREATION
support/setup-dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4
support/setup-dev.sh PRE-CREATION
Diff: https://reviews.apache.org/r/71205/diff/10/
Changes: https://reviews.apache.org/r/71205/diff/9-10/
Testing
-------
* used successfully for a couple of months
Thanks,
Benjamin Bannier
Re: Review Request 71205: Switched commit hooks to pre-commit.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/#review217461
-----------------------------------------------------------
Fix it, then Ship it!
support/setup-dev.bat
Lines 59 (patched)
<https://reviews.apache.org/r/71205/#comment304726>
s/mlink/mklink/
- Joseph Wu
On Aug. 27, 2019, 1:08 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2019, 1:08 a.m.)
>
>
> Review request for mesos and Till Toenshoff.
>
>
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
>
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
>
>
> Diffs
> -----
>
> docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c
> support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b
> support/pre-commit-config.yaml PRE-CREATION
> support/setup-dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4
> support/setup-dev.sh PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/71205/diff/9/
>
>
> Testing
> -------
>
> * used successfully for a couple of months
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71205: Switched commit hooks to pre-commit.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/
-----------------------------------------------------------
(Updated Aug. 27, 2019, 10:08 a.m.)
Review request for mesos and Till Toenshoff.
Changes
-------
Address comments from Till
Bugs: MESOS-9630
https://issues.apache.org/jira/browse/MESOS-9630
Repository: mesos
Description
-------
This patch switches commit hooks to be orchestrated by the pre-commit
tool mirroring the previous linters invoked through git commit
hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
Using pre-commit removes the burden of maintaining
`support/mesos-style.py`, making sure that hooks have the expected
environment (e.g., Python version, Node installed). Additionally,
upstream provides a number of additional linters which are not hard to
add to Mesos' hooks.
Diffs (updated)
-----
docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c
support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b
support/pre-commit-config.yaml PRE-CREATION
support/setup-dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4
support/setup-dev.sh PRE-CREATION
Diff: https://reviews.apache.org/r/71205/diff/9/
Changes: https://reviews.apache.org/r/71205/diff/8-9/
Testing
-------
* used successfully for a couple of months
Thanks,
Benjamin Bannier
Re: Review Request 71205: Switched commit hooks to pre-commit.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/
-----------------------------------------------------------
(Updated Aug. 20, 2019, 1:48 p.m.)
Review request for mesos and Till Toenshoff.
Changes
-------
Rebase
Bugs: MESOS-9630
https://issues.apache.org/jira/browse/MESOS-9630
Repository: mesos
Description
-------
This patch switches commit hooks to be orchestrated by the pre-commit
tool mirroring the previous linters invoked through git commit
hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
Using pre-commit removes the burden of maintaining
`support/mesos-style.py`, making sure that hooks have the expected
environment (e.g., Python version, Node installed). Additionally,
upstream provides a number of additional linters which are not hard to
add to Mesos' hooks.
Diffs (updated)
-----
docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c
support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b
support/pre-commit-config.yaml PRE-CREATION
support/setup-dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4
support/setup-dev.sh PRE-CREATION
Diff: https://reviews.apache.org/r/71205/diff/8/
Changes: https://reviews.apache.org/r/71205/diff/7-8/
Testing
-------
* used successfully for a couple of months
Thanks,
Benjamin Bannier
Re: Review Request 71205: Switched commit hooks to pre-commit.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71205/
-----------------------------------------------------------
(Updated Aug. 19, 2019, 9:20 a.m.)
Review request for mesos and Till Toenshoff.
Changes
-------
Rebase; install config via link to allow for untracked modifications
Bugs: MESOS-9630
https://issues.apache.org/jira/browse/MESOS-9630
Repository: mesos
Description
-------
This patch switches commit hooks to be orchestrated by the pre-commit
tool mirroring the previous linters invoked through git commit
hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
Using pre-commit removes the burden of maintaining
`support/mesos-style.py`, making sure that hooks have the expected
environment (e.g., Python version, Node installed). Additionally,
upstream provides a number of additional linters which are not hard to
add to Mesos' hooks.
Diffs (updated)
-----
docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c
support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b
support/pre-commit-config.yaml PRE-CREATION
support/setup-dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4
support/setup-dev.sh PRE-CREATION
Diff: https://reviews.apache.org/r/71205/diff/7/
Changes: https://reviews.apache.org/r/71205/diff/6-7/
Testing
-------
* used successfully for a couple of months
Thanks,
Benjamin Bannier