-
-
Notifications
You must be signed in to change notification settings - Fork 115
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Reorganize contributing docs + add process description. (#3044)
* Reorganize contributing docs + add process description. * Update conda-lock.yml and rendered conda environment files. * Update for PR comments --------- Co-authored-by: jdangerx <[email protected]>
- Loading branch information
Showing
10 changed files
with
272 additions
and
197 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,30 @@ | ||
| --- | ||
| name: New dataset | ||
| about: Provide information about a new dataset you'd like to see in PUDL | ||
| title: '' | ||
| labels: new-data | ||
| 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? | ||
|
|
||
| ### Logistics | ||
|
|
||
| Is this dataset publically available? | ||
|
|
||
| Where can one download the actual data? | ||
|
|
||
| How often does this dataset get updated? | ||
|
|
||
| What licensing restrictions apply? | ||
|
|
||
| ### 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,25 @@ | ||
| <!-- | ||
| 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 | ||
|
|
||
| # PR Checklist | ||
| How did you make sure this worked? How can a reviewer verify this? | ||
|
|
||
| - [ ] 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] | ||
| # To-do list | ||
| - [ ] 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: make sure to update or write data validation tests | ||
| - [ ] 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
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,100 @@ | ||
| ------------------------- | ||
| Contributing Code to PUDL | ||
| ------------------------- | ||
|
|
||
| Welcome! We're so glad you're interested in contributing to PUDL! We would love | ||
| some help making PUDL data as complete as possible. | ||
|
|
||
| .. _after-intro: | ||
|
|
||
| .. IMPORTANT:: Already have a dataset in mind? | ||
|
|
||
| If you **need data that's not in PUDL**, `open an issue | ||
| <https://github.com/catalyst-cooperative/pudl/issues/new?assignees=&labels=new-data&projects=&template=new_dataset.md&title=>`__ | ||
| to tell us more about it! | ||
|
|
||
| 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. | ||
|
|
||
|
|
||
| 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 in our `Community Kanban Board | ||
| <https://github.com/orgs/catalyst-cooperative/projects/9/views/19>`__. These | ||
| are issues that don't require a ton of PUDL-specific context, and are | ||
| relatively tightly scoped. | ||
|
|
||
| * Comment on the issue and tag ``@catalyst-cooperative/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! Post a comment outlining: | ||
| * what steps have you broken this down into? | ||
| * what is the output of each step? | ||
| * how will one know that each step is working? | ||
|
|
||
| * Once you've talked through your plan with someone from Catalyst, go forth and | ||
| develop! | ||
|
|
||
| **Work on it!** | ||
|
|
||
| * Make a branch on your fork and open a draft pull request (PR) early so we can | ||
| discuss concrete code! **Set the base branch to ``dev`` unless there's a good | ||
| reason otherwise.** 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! | ||
| We automatically run the test suite for all PRs, but some of those will have | ||
| to be manually approved by Catalyst members for safety reasons. | ||
|
|
||
| * **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. We try to keep PRs to **less than 500 lines of code.** | ||
|
|
||
| **Get it merged in!** | ||
|
|
||
| * Turn the draft PR into a normal PR and tag ``@catalyst-cooperative/com-dev`` | ||
| in a comment. We'll try to get back to you within a few days - the | ||
| smaller/simpler the PR, the faster we'll be able to get back to you. | ||
|
|
||
| * The reviewer will leave comments - if they request changes, address their | ||
| concerns and re-request review. | ||
|
|
||
| * There will probably be some back-and-forth until your PR is approved - this | ||
| is normal and a sign of good communication on your part! Don't be shy about | ||
| asking us for updates and re-requesting review! | ||
|
|
||
| * Don't 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 | ||
| participants can see your comments (they are invisible to others if marked | ||
| "Pending"). | ||
|
|
||
| Next contributions | ||
| ------------------ | ||
|
|
||
| Hooray! You made your first contribution! To find another issue to tackle, check | ||
| out the `Community Kanban board | ||
| <https://github.com/orgs/catalyst-cooperative/projects/9/views/19>`__ where | ||
| we've picked out some issues that are | ||
|
|
||
| * useful to work on | ||
|
|
||
| * unlikely to become super time-sensitive | ||
|
|
||
| * have some context, success criteria, and next steps information. | ||
|
|
||
| Pick one of these and follow the contribution flow 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
Oops, something went wrong.