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 tohttps://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
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 <yt-dev@python.org> 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
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 <yt-dev@python.org> 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
Hi Corentin, thanks for your reply !
On 11 Jan 2021, at 14:30, Corentin CADIOU <corentin.cadiou@iap.fr> wrote:
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.
Agreed
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?
Good question ! For hooks that merely report issues rather than solve them, we would get very similar results as what we’re getting now with GH actions: A failed check. To go one step further, the pre-commit-ci bot will run our hooks each time a new commit is added, including one of its own. It fails if one or more hooks report failures, and it produces a new commit if and only if at least one hook generates changes. Note that there could exist some niche cases where different hooks fight each other and the bot would produce more than one commit. I’ve identified exactly ONE such case since we deployed hooks, were black -> flynt can produce unstable results. This is trivially solved by inverting the order of operations. Clément
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 <yt-dev@python.org> 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
_______________________________________________ yt-dev mailing list --yt-dev@python.org To unsubscribe send an email toyt-dev-leave@python.org https://mail.python.org/mailman3/lists/yt-dev.python.org/ Member address:clement.robert@protonmail.com <OpenPGP_signature.sig>
participants (3)
-
Clément Robert -
Corentin CADIOU -
Matthew Turk