Hello again, I have landed the patches for MESOS-9630 on the `master` branch, so we now use pre-commit as linting framework.
pre-commit primer ================= 0. Install pre-commit, https://pre-commit.com/#install. 1. Run `./support/setup-dev.sh` to install hooks. We have broken developer-related setup out of `./bootstrap` which by now only bootstraps the autotools project while `support/setup-dev.sh` sets up developer configuration files and git hooks. 2. As git hooks are global to a checkout and not tied to branches, you might run into issues with the linter setup on older branches since configuration files or scripts might not be present. You either should setup that branch's linters with e.g., `./bootstrap`, or could silence warnings from the missing linter setup with e.g., $ PRE_COMMIT_ALLOW_NO_CONFIG=1 git commit 3. You can use the `SKIP` environment variable to disable certain linters, e.g., # git-revert(1) often produces titles longer than 72 characters. $ SKIP=gitlint git revert HEAD `SKIP` takes a linter `id` which you can look up in `.pre-commit-config.yaml`. 4. We still use git hooks, but To explicitly lint your staged changes before a commit execute # Run all applicable linters, $ pre-commit # or a certain linter, e.g., `cpplint`. $ pre-commit run cpplint pre-commit runs only on staged changes. 5. To run a full linting of the whole codebase execute $ SKIP=split pre-commit run -a We need to skip the `split` linter as it would complain about a mix of files from stout, libprocess, and Mesos proper (it could be rewritten to lift this preexisting condition). 6. pre-commit caches linter environments in `$XDG_CACHE_HOME/.pre-commit` where `XDG_CACHE_HOME` is most often `$HOME/.cache`. While pre-commit automatically sets up linter environments, cleanup is manual # gc unused linter environments, e.g., after linter updates. $ pre-commit gc # Remove all cached environments. $ pre-commit clean 7. To make changes to your local linting setup replace the symlink `.pre-commit-config.yaml` with a copy of `support/pre-commit-config.yaml` and adjust as needed. pre-commit maintains a listing of hooks of varying quality, https://pre-commit.com/hooks.html and other linters can be added pretty easily (see e.g., the `local` linters `split`, `license`, and `cpplint` in our setup). Consider upstreaming whatever you found useful. Happy linting, Benjamin On Sat, Aug 17, 2019 at 2:12 PM Benjamin Bannier <bbann...@apache.org> wrote: > Hi, > > I opened MESOS-9360[^1] to improve the way we do linting in Mesos some time > ago. I have put some polish on my private setup and now published it, and > am > asking for feedback as linting is an important part of working with Mesos > for > most of you. I have moved my workflow to pre-commit more than 6 months ago > and > prefer it so much that I will not go back to `support/mesos-style.py`. > > * * * > > We use `support/mesos-style.py` to perform linting, most often triggered > automatically when committing. This setup is powerful, but also hard to > maintain and extend. pre-commit[^2] is a framework for managing Git commit > hooks which has an exciting set of features, one can often enough > configure it > only with YAML and comes with a long list of existing linters[^3]. Should > we > go with this approach we could e.g., trivially enable linters for Markdown > or > HTML (after fixing the current, sometimes wild state of the sources). > > I would encourage you to play with the [chain] ending in r/71300[^4] on > some > fresh clone (as this modifies your Git hooks). You need to install > pre-commit[^5] _before applying the chain_, and then run > `support/setup_dev.sh`. This setup mirrors the existing functionality of > `support/mesos-style.py`, but also has new linters activated. This should > present a pretty streamlined workflow. I have also adjusted the Windows > setup, > but not tested it. > > I have also spent some time to make transitioning from our current linting > setup easier. If you are feeling adventurous you can apply the chain up to > r/71209/ on your existing setup and run `support/setup_dev.sh`. > > One noticeable change is that with pre-commit we will store (some) linters > in > `$XDG_CACHE_HOME` (default: `$HOME/.cache`). The existing setup stores some > linter files in the build directory, so a "clean build" might require > downloading linter files again. With pre-commit OTOH one needs to perform > garbage-collection out of band (e.g., by executing `pre-commit gc`, or > deleting > the cache directory). > > * * * > > Please let me know whether we should move forward with this change, you > think > it needs important adjustments, or you see fundamental reasons that this > is a > bad idea. If you like what you see here I would be happy to know about > that as > well. > > > Cheers, > > Benjamin > > > [^1]: https://issues.apache.org/jira/browse/MESOS-9630 > [^2]: https://pre-commit.com/ > [^3]: https://pre-commit.com/hooks.html > [^4]: https://reviews.apache.org/r/71300/ > [^5]: https://pre-commit.com/#install > [^6]: https://reviews.apache.org/r/71209 >
