-
Notifications
You must be signed in to change notification settings - Fork 69
ci: rely on pre-commit.ci & run tests only if needed #175
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
ci: rely on pre-commit.ci & run tests only if needed #175
Conversation
Note that the pre-commit.ci service is configured to run against individual repo's in the JupyterHub org currently, pratically those that have .pre-commit-config.yaml files setup within them. This is configured in https://github.com/organizations/jupyterhub/settings/installations/17782968. Currently, pre-commit.ci service is activated to run for the following JupyterHub org repos:
|
I'm not sure if switching to an external vendor over GitHub actions is worth it in this case, tbh. I do like having it all integrated in one platform (GitHub). So, what exactly are the upsides here over using GitHub actions? So far I've read about "speed" (not really an issue, imho, since the tests already run decently fast) and the addition of fix-commits to PRs. But wouldn't any needed fix in PRs already run through the pre-commit hooks on merge, or, at the latest, on the next commit? |
I pushed back for the same reasons initially, it adds complexity to have another service (jupyterhub/team-compass#379 (comment)). Here are two examples of the pre-commit.ci service coming to use in a way that makes me positive about accepting that added complexity.
For me that are now used to pre-commit.ci being used in other repos, the added complexity to use it in another repo is low as I've already embraced that complexity in other repos. |
If you're convinced it will be a positive addition, I'm willing to give this a try. A testing phase, so to speak. We can revert later, if needed. But if we're giving this a go, we should probably also add it to the requirements as per here, right? Maybe also say a sentence or two on how to run these on your own in the README as well? |
Alright, let's give this a go! |
@lambdaTotoro I'm glad you are willing to try it out, I'll be quick to approve if you think it's for the better to revert to using github workflows to do this check later! I pushed a commit documenting it! |
Haha wow that was quick :D ❤️ 🎉 |
Now with a .pre-commit-config.yaml and changes made so that the pre-commit checks pass in #173 and #174, we can adopt the service https://pre-commit.ci to help us run these tests with very high speed together with automation to apply auto formatting for those that hasn't installed and let pre-commit run before creating a PR.
The use of pre-commit.ci has been considered in jupyterhub/team-compass#379. I've found the service to be really helpful so far, and there seems to be some tentative agreement towards that.