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