-
-
Notifications
You must be signed in to change notification settings - Fork 256
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
Add a GitHub PR template #814
base: main
Are you sure you want to change the base?
Conversation
.github/PULL_REQUEST_TEMPLATE.md
Outdated
@@ -0,0 +1,4 @@ | |||
- [ ] Closes #xxxx | |||
- [ ] Tests added / passed | |||
- [ ] Passes `black dask_ml` / `flake8 dask_ml` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll probably want to add isort
and mypy
to this check too. At a certain point it might be worth pointing to ci/code_checks.sh
or pre-commit
instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll probably want to add isort and mypy to this check too.
I was waiting for the resolution of #813 first, before introducing an isort
check here, but now that you've brought it up, I don't see why it couldn't be introduced at the current stage.
Thanks for reminding me about mypy
- I'd completely forgotten about it.
At a certain point it might be worth pointing to ci/code_checks.sh or pre-commit instead
Do you mean verbally, or is there some related GitHub magic trick(s)?
bb25224
to
34e41eb
Compare
@@ -0,0 +1,8 @@ | |||
- [ ] Closes #xxxx | |||
- [ ] Tests added / passed | |||
- [ ] Passes `flake8` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opted to:
- follow the checks as per https://github.com/dask/dask-ml/blob/main/ci/code_checks.sh (e.g., and not do
flake8 dask_ml
instead of justflake8
); - split the individual code checks into several lines.
Would be happy to amend the PR accordingly, subject to expressed preferences for alternatives.
Hmm the "CI / docs (pull_request)" failure is new:
It doesn't seem to me that it's related to the most recent commit pushed to this PR. |
I can see the same failure in #813. @jrbourbeau, could this by any chance be related to dask/distributed#4647 (comment)? |
Ah, thanks for pointing the documentation build errors out -- should be fixed by #815 |
This PR proposes the introduction of a PR template, following the same structure as those in dask and distributed.