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

Consider pre-commit-ci/lite #104

Closed
jl-wynen opened this issue Nov 23, 2023 · 5 comments · Fixed by #120
Closed

Consider pre-commit-ci/lite #104

jl-wynen opened this issue Nov 23, 2023 · 5 comments · Fixed by #120

Comments

@jl-wynen
Copy link
Member

If I understand correctly, we could use https://github.com/pre-commit-ci/lite-action instead of the autocommit action to apply changes to PRs. This may work better than the current approach which creates a separate commit but doesn't run CI on it.
We'd still have to run pre-commit ourselves.

@YooSunYoung
Copy link
Member

YooSunYoung commented Dec 14, 2023

Let's try this! Should we install it for one of repository and try there first?
I can volunteer to try it in essnmx for example.
I think it should be an owner of organization to install them and give permission...?

@jl-wynen
Copy link
Member Author

I just installed it for essnmx.

@YooSunYoung
Copy link
Member

Here is what I tested in essnmx: scipp/essnmx#12.

Applied wrong format on purpose and pre-commit fixed them

image

Pre-commit CI made a commit with fixes

image

Final CI tests passes!

image

@YooSunYoung
Copy link
Member

YooSunYoung commented Dec 15, 2023

I think it's very neat and we should use this one instead.
First we need to install them for all repositories I guess,
and then I can make the change in the template.

@jl-wynen
Copy link
Member Author

And it actually ran the CI pipeline on the new commit!

I'm happy with it, any objections @scipp/scipp-maintainers ?

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 a pull request may close this issue.

2 participants