-
-
Notifications
You must be signed in to change notification settings - Fork 118
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* call out specific common cases * pull dev-specific CONTRIBUTING.rst into top-level * add 'new dataset' issue template * hopefully make PR template less overwhelming
- Loading branch information
Showing
5 changed files
with
146 additions
and
142 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
--- | ||
name: New dataset | ||
about: Provide information about a new dataset you'd like to see in PUDL | ||
title: '' | ||
labels: new-dataset | ||
assignees: '' | ||
--- | ||
|
||
### Overview | ||
|
||
What is this dataset? Why do you want it in PUDL? Is it already partially in | ||
PUDL, or do we need to start from scratch? | ||
|
||
### Where is it? | ||
|
||
Is this dataset publically available? Where can one download the actual data? | ||
|
||
### What do you know about it so far? | ||
|
||
What have you done with this dataset so far? Have you run into any problems with | ||
it yet? |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,49 +1,26 @@ | ||
<!-- | ||
Making a PUDL Pull Request | ||
Before making a PR you may want to check out our: | ||
Resources: | ||
* contributing guidelines: https://catalystcoop-pudl.readthedocs.io/en/latest/CONTRIBUTING.html | ||
* code of conduct: https://catalystcoop-pudl.readthedocs.io/en/latest/code_of_conduct.html | ||
* development process: https://catalystcoop-pudl.readthedocs.io/en/latest/dev/index.html | ||
## PR Process Overview | ||
* PRs have to get an approving review before merging into their development branch. | ||
* Most PRs should be made against the `dev` branch, unless they are part of some larger ongoing refactoring, in which case there will be a persistent development branch for that work. | ||
* It is much easier to do timely code reviews on smaller chunks of code. We try to keep PRs under 500 lines of code. | ||
* Draft PRs are a good way to get early feedback on designs or several incremental commits that will add up to larger changes. If you want a review of a draft PR, make sure you contact the reviewer directly or mention their username in the PR comment, so they get a notification. | ||
* How quickly we can review a PR will depend on how large and complex it is, and how busy we are, but ideally we strive to get an initial review done within a week. If there are going to be delays, we should at least comment on the PR to let you know the situation. | ||
* If you believe you've addressed a reviewer's comments, respond with a brief note and mark the comment resolved. If further discussion is requried respond and do not resolve the comment. | ||
* Before a PR is merged all reviewer comments should be resolved. If a reviewer doesn't feel that their comment has been sufficiently addressed, they may unresolve a comment. | ||
* Be careful not to accidentally "start a review" when responding to comments! If this does happen, don't forget to submit the review you've started so the other PR participatns can see your comments (they are invisible to others if marked "Pending"). | ||
* In the period after an initial review when there is significant back-and-forth with the reviewer deciding what changes should actually be made, there should probably be daily interaction. If significant changes are required, it's usually best to request another review after those changes have been made. | ||
Feel free to delete the commented-out parts of the template before submitting the PR. | ||
--> | ||
# Overview | ||
|
||
# PR Overview | ||
Closes #XXXX. | ||
|
||
<!-- | ||
What problem does this address? | ||
|
||
Include a short narrative summary of what's going on in the PR. This can be a bulleted list. You might want to include: | ||
What did you change? | ||
|
||
* What are you changing and why? | ||
* Are there any known unsolved problems remaining in the PR? | ||
* Is there anything that you want a reivewer to pay particular attention to? | ||
* What kind of feedback are you looking for on the PR? | ||
# Testing | ||
|
||
--> | ||
How did you make sure this worked? How can a reviewer verify this? | ||
|
||
# PR Checklist | ||
|
||
- [ ] Merge the most recent version of the branch you are merging into (probably `dev`). | ||
- [ ] All CI checks are passing. [Run tests locally to debug failures](https://catalystcoop-pudl.readthedocs.io/en/latest/dev/testing.html#running-tests-with-tox) | ||
- [ ] Make sure you've included good docstrings. | ||
```[tasklist] | ||
# Remaining work | ||
- [ ] Make sure full ETL runs & `make pytest-integration-full` passes locally | ||
- [ ] For major data coverage & analysis changes, [run data validation tests](https://catalystcoop-pudl.readthedocs.io/en/latest/dev/testing.html#data-validation) | ||
- [ ] Include unit tests for new functions and classes. | ||
- [ ] Defensive data quality/sanity checks in analyses & data processing functions. | ||
- [ ] Update the [release notes](https://catalystcoop-pudl.readthedocs.io/en/latest/release_notes.html) and reference reference the PR and related issues. | ||
- [ ] Do your own explanatory review of the PR to help the reviewer understand what's going on and identify issues preemptively. | ||
- [ ] If updating analyses or data processing functions: write data quality checks | ||
- [ ] Update the [release notes](../docs/release_notes.rst): reference the PR and related issues. | ||
- [ ] Review the PR yourself and call out any questions or issues you have | ||
``` | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
Contributing to PUDL | ||
==================== | ||
|
||
Help get more data into PUDL! | ||
|
||
|
||
.. IMPORTANT:: Already have a dataset in mind? | ||
|
||
If you **need data that's not in PUDL** that we're missing in PUDL, | ||
`open an issue <https://github.com/catalyst-cooperative/pudl/issues/new/choose>`__. | ||
|
||
If you've **already written some code to wrangle a dataset**, find us at | ||
`office hours <https://calend.ly/catalyst-cooperative/pudl-office-hours>`__ and we | ||
can talk through next steps for how to get that into PUDL. | ||
|
||
Your first contribution | ||
----------------------- | ||
|
||
**Setup** | ||
|
||
You'll need to fork this repository and get the | ||
`dev environment set up <https://catalystcoop-pudl.readthedocs.io/en/latest/dev/dev_setup.html>`__. | ||
|
||
**Pick an issue** | ||
|
||
* Look for issues with the `good first issue | ||
<https://github.com/catalyst-cooperative/pudl/issues?q=is%3Aissue+is%3Aopen+label%3Agood-first-issue>`__ | ||
tag. These are issues that don't require a ton of PUDL-specific context, and | ||
are relatively tightly scoped to boot. | ||
|
||
* Comment on the issue and tag ``@com-dev`` (our Community Development Team) to | ||
let us know you're working on it. Feel free to ask any questions you might | ||
have! | ||
|
||
* Once you have an idea of how you want to tackle this issue, write out your | ||
plan so we can guide you around obstacles in your way. | ||
|
||
**Work on it!** | ||
|
||
* Make a branch on your fork and open a draft PR early so we can discuss | ||
concrete code! Please don't wait until it's all polished up - it's much easier | ||
for us to help you when we can see the code evolve over time. | ||
|
||
* Please make sure to write tests and documentation for your code - if you run | ||
into trouble with writing tests, let us know in the comments and we can help! | ||
|
||
* Please try to keep your changes relatively small: stuff happens, and one's | ||
bandwidth for volunteer work can fluctuate frequently. If you make a bunch of | ||
small changes, it's much easier to pause on a project without losing a ton of | ||
context. | ||
|
||
**Get it merged in!** | ||
|
||
* Turn the draft PR into a normal PR and ping ``@com-dev``. We'll try to get | ||
back to you within a few days. | ||
|
||
**Next contributions** | ||
|
||
Any issues with the `community | ||
<https://github.com/catalyst-cooperative/pudl/issues?q=is%3Aissue+is%3Aopen+label%3Acommunity>`__ | ||
tag are up for grabs! Follow the same process as above. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters