> 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
> 
>

Reply via email to