Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pre-commit.ci] pre-commit autoupdate hooks #1848

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.4.3 → v0.4.7](astral-sh/ruff-pre-commit@v0.4.3...v0.4.7)
- [github.com/thibaudcolas/pre-commit-stylelint: v16.5.0 → v16.6.1](thibaudcolas/pre-commit-stylelint@v16.5.0...v16.6.1)
@drammock drammock mentioned this pull request Jun 3, 2024
@drammock
Copy link
Collaborator

drammock commented Jun 3, 2024

@trallard @Carreau AFAICT, the pre-commit CI App and our local pre-commit hooks (run in the CI's lint job) are fighting each other. I don't have sufficient permissions to configure the repo-level app (https://github.com/pydata/pydata-sphinx-theme/settings/installations)

I think the right thing to do is to ignore CIs on this PR and just merge it, and then merge main into e.g. #1850 and hope that the pre-commit CI stops correcting files in a way that makes the lint job fail.

to repro locally the CI lint job failure:

pre-commit clean
tox -re lint

cc @MridulS

@Carreau
Copy link
Collaborator

Carreau commented Jun 3, 2024

I dont' have enough permission either.

My take is what is ran by pre-commit should not be ran in GH action.

From a few exchange iI had today it seem that a few people are frustrateed by tox+gh-action.

@drammock
Copy link
Collaborator

drammock commented Jun 3, 2024

the pre-commit CI action is pretty useful for two cases:

  1. making edits to a PR through the GitHub UI, where pre-commit isn't otherwise run
  2. PRs opened by first-time contributors who haven't installed the pre-commit hooks

But anyway since neither of us have permissions to configure/disable, for now maybe let's merge this and hope that it resolves the fight between the action and the hook. WDYT?

@drammock drammock merged commit 74593a4 into main Jun 3, 2024
8 of 9 checks passed
@drammock drammock deleted the pre-commit-ci-update-config branch June 3, 2024 23:59
@trallard
Copy link
Collaborator

trallard commented Jun 4, 2024

The easiest is to remove the pre-commit from the CI on GH actions. I just left it there since it was already in our CI and it did not seem ppl had much trouble at the time.

Separate I had already asked folks at NUMFOCUS (since the repo is in the PyData org) while talking to them in person recently to grant maintainers of the theme elevated access to the repo as we keep hitting blockers to change configs and the likes so we have had to resource to ping ppl like Andy Terrel.
Shall we surface this again perhaps in the projects channel in the NF Slack?

@drammock
Copy link
Collaborator

drammock commented Jun 4, 2024

Shall we surface this again perhaps in the projects channel in the NF Slack?

yes please!

cc @jarrodmillman

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants