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: fix ci failure, optimize workflow triggers, document use of pre-commit.ci #477

Merged
merged 3 commits into from
Feb 16, 2022

Conversation

consideRatio
Copy link
Collaborator

@consideRatio consideRatio commented Feb 12, 2022

Ignore this PR until having considered #476!

If there is agreement on using the pre-commit.ci service as motivated within #476, then this PR closes #476.

branches-ignore:
- "dependabot/**"
- "pre-commit-ci-update-config"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pre-commit.ci / dependabot pushes to these branches and opens a PR. So like this one would avoid triggering two workflow executions - one for the pushed branch, one for the PR.

@martindurant
Copy link
Member

martindurant commented Feb 12, 2022

I do like normalising how things are done, and of course fast failures - all without worrying about upstream checker versions. Does this add any extra effort when developing locally?

@consideRatio
Copy link
Collaborator Author

Does this add any extra effort when developing locally?

Using pre-commit.ci would remove one piece of extra effort of developing locally and submitting a PR, which is to install and run pre-commit, which in turn run the black autoformatting hook as configured already in this repo. This is because pre-commit.ci would push a commit with the autoformatting changes needed to the PR if you had not done it yourself locally.

Whats critical and should be fixed no matter what, is that we now have a .pre-commit-config.yaml referencing an outdated version of black, and then we run tests with a new version of black in the github workflow.

@martindurant
Copy link
Member

I see what you mean :)

would reformat dask-gateway/dask_gateway/options.py
would reformat dask-gateway-server/dask_gateway_server/backends/jobqueue/pbs.py
would reformat dask-gateway-server/dask_gateway_server/backends/jobqueue/slurm.py
would reformat dask-gateway-server/dask_gateway_server/backends/kubernetes/utils.py
would reformat dask-gateway-server/dask_gateway_server/traitlets.py
would reformat dask-gateway-server/dask_gateway_server/workqueue.py
would reformat dask-gateway-server/dask_gateway_server/utils.py
would reformat tests/kubernetes/test_methods.py
would reformat tests/test_pbs_backend.py
would reformat tests/test_slurm_backend.py
would reformat tests/test_utils.py

Oh no! 💥 💔 💥
[11](https://github.com/dask/dask-gateway/runs/5168260835?check_suite_focus=true#step:9:11) files would be reformatted, 57 files would be left unchanged.

@jacobtomlinson
Copy link
Member

Test failures here are unrelated. Merging.

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.

CI: avoid and fix ci failure, either by relying on pre-commit.ci or running pre-commit in a github workflow
3 participants