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

Use setup-python instead of setup-micromamba for pre-commit CI #388

Merged
merged 1 commit into from
Oct 13, 2024

Conversation

RMeli
Copy link
Member

@RMeli RMeli commented Oct 11, 2024

@RMeli
Copy link
Member Author

RMeli commented Oct 11, 2024

We had a failure in #387. Trying to see if using setup-python works.

Others encountered the same issue pre-commit/action#214, pre-commit/action#213, pre-commit/action#210. Honestly, the replies from the maintainer on these issues makes me want to steer clear of the pre-commit GitHub action.

Additionally:

this action is in maintenance-only mode and will not be accepting new features.
generally you want to use pre-commit.ci which is faster and has more features.

Maybe we should consider moving to pre-commit.ci? @jandom?

@RMeli RMeli marked this pull request as ready for review October 11, 2024 20:34
Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know very little about the background here. I am providing a "LGTM" because it looks sensible to use a simple python setup to run pre-commit and because you are of the opinion that it will improve the CI and because it didn't visibly break anything.

If someone with more experience could have a look then that would be good, of course, but I ultimately trust that you know what you're doing.

.github/workflows/pre-commit.yaml Show resolved Hide resolved
@RMeli
Copy link
Member Author

RMeli commented Oct 11, 2024

I could not find much information on why this is happening now. But it seems that the action is only in maintenance mode, and the maintainer is very quick at closing issues. In the ones outlining this problem (linked above, links were wrong and I just updated them), replies are of the sort

you are the third or fourth person to not search before creating an issue for this specific thing. please do better!

or

skill issue imo. it is in the readme you just didn't follow it or read the output of your own job

So not much information there. The README uses the setup-python action to setup Python for pre-commit, so that's what I went for and it seems to work as expected.

@orbeckst
Copy link
Member

The maintainer comments on the pre-commit issues are not particularly welcoming. I don't quite know what would be better for a "maintenance-only" project: no response at all or harsh responses + shutting down issues. 🤷

Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like the precomit action is doing something reasonably nefarious.. I do agree that moving away from it would be good.

@RMeli RMeli merged commit 10eafba into develop Oct 13, 2024
5 checks passed
@RMeli RMeli deleted the pre-commit branch October 13, 2024 10:33
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