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

ci: rely on pre-commit.ci & run tests only if needed #175

Merged
merged 3 commits into from
Oct 31, 2021

Conversation

consideRatio
Copy link
Member

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.

@consideRatio
Copy link
Member Author

consideRatio commented Oct 31, 2021

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:

  • jupyterhub/jupyterhub
  • jupyterhub/oauthenticator
  • jupyterhub/kubespawner
  • jupyterhub/zero-to-jupyterhub-k8s
  • jupyterhub/ltiauthenticator
  • jupyterhub/docker-image-cleaner

@lambdaTotoro
Copy link
Collaborator

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?

@consideRatio
Copy link
Member Author

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.

@lambdaTotoro
Copy link
Collaborator

lambdaTotoro commented Oct 31, 2021

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?

@lambdaTotoro
Copy link
Collaborator

Alright, let's give this a go!

@consideRatio
Copy link
Member Author

@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!

@lambdaTotoro lambdaTotoro merged commit 75b771b into jupyterhub:main Oct 31, 2021
@consideRatio
Copy link
Member Author

Haha wow that was quick :D ❤️ 🎉

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

Successfully merging this pull request may close these issues.

None yet

2 participants