Hi Clément, Sorry for taking so long to get back to you! It sounds like a really good idea to me. There are many PRs whose tests don't pass because of these formatting issues (e.g. when accepting modifications directly on the Github interface). Having an automatic way of fixing these issues would certainly streamline to workflow and lower the barrier for new contributors. Regarding the disruption of users workflow, I think that advising `git push --force` for contributors with little experience of git is a really good solution, with the extra benefit that the automatic formatting commit would only be included once in the list of commits to be considered. Once thing I am wondering is for flake8, since it shows issues but doesn't solve them. How would that be included in the workflow? Cheers, Corentin On 28/12/2020 18:27, Matthew Turk wrote:
Hi Clément, this definitely sounds worth a shot to me. It would simplify us maintaining github actions all over the place, too, which was *fun* but also bore with it the promise of *less fun later*. So, yeah!
On Mon, Dec 28, 2020 at 4:43 AM Clément Robert via yt-dev
wrote: Dear yt-dev
I recently found out about https://pre-commit.ci, and I'd like to propose we adopt this tool as part of our CI for linting/auto-fixing.
The idea is that this automatically runs
$ pre-commit run --all-files
on each pull request and commits to the corresponding branch to auto-fix errors with pre-commit hooks (black, isort and flynt in our case). I'm reaching out to you because this would have small repercusions on everyone's workflow. Let me try to give a comprehensive description of gains and costs in the following.
what it gets us
a simplification of our CI tooling, namely: - `tests/lint_requirements.txt` - `.github/workflows/style-checks.yaml` - https://github.com/yt-project/slash-command-processor all become useless
`.github/PULL_REQUEST_TEMPLATE.yaml` will also be simplified as all linting items in `PR Checklist` become redundant.
On top of this, new hooks could be seamlessly added in the future without increasing the entry barrier for occasional contributors (as installing pre-commit hooks locally is and remains an opt-in).
what it costs us
- "zero" configuration: the only requirement is `.pre-commit.yaml`, which is already part of the repo. - minor updates to .pre-commit-config.yaml, namely : - bumping black's version (reason: black's `--exclude` flag is overidden by pre-commit's `--all-files`, but more recent versions of black have a `--force-exclude` flag that overcomes this) this will require a new PR to adopt black's latest (minor) updates in code styling - signing up to https://pre-commit.ci (free)
what this changes to your workflow
PRs will be automatically fixed by the bot, so you'll either need to occasionally run `git pull` or `git push --force` to account for it. Note that this can be avoided completely by installing pre-commit hooks locally which is and will remain an opt-in, see https://yt-project.org/docs/dev/developing/developing.html#pre-commit-hooks.
Let me know what you think. Take care, and happy new year ! Cheers, Clément _______________________________________________ yt-dev mailing list -- yt-dev@python.org To unsubscribe send an email to yt-dev-leave@python.org https://mail.python.org/mailman3/lists/yt-dev.python.org/ Member address: matthewturk@gmail.com
yt-dev mailing list -- yt-dev@python.org To unsubscribe send an email to yt-dev-leave@python.org https://mail.python.org/mailman3/lists/yt-dev.python.org/ Member address: corentin.cadiou@iap.fr
-- Dr. Corentin Cadiou Post Doctoral Research Assistant Cosmoparticle Initiative Hub, desk 27 University College London (UCL) Gower St, Bloomsbury, London WC1E 6BT mobile: +33.6.43.18.66.83