Add pull request template #50
Conversation
santisoler
left a comment
santisoler
left a comment
There was a problem hiding this comment.
Hi @dccowan! This looks great! I just left a few comments below that would improve the rendering of the Markdown, fix some undefined references, and some minor corrections to the text.
I think the biggest comment I have is regarding the requirements for contributions. Find them below in one the comments, feel free to reply there.
| In this cell, the contributor must provide a set of relevant keywords. E.g. | ||
|
|
||
| ``` | ||
| **Keywords:** gravity inversion, sparse-norm inversion, integral formulation, tree mesh. |
There was a problem hiding this comment.
Just sharing an idea, not to tackle it now: MystMD has support for specifying keywords through the frontmatter. I think at some point it would be better to use those instead of hard-coding the keywords as Markdown text.
# Conflicts: # notebooks/03-gravity/fwd_gravity_gradiometry_3d.ipynb
Co-authored-by: Santiago Soler <santisoler@fastmail.com>
Co-authored-by: Santiago Soler <santisoler@fastmail.com>
…r-tutorials into pull_request_template # Conflicts: # notebooks/contributing_index.md
|
@lheagy @santisoler I think I have finalized the PR for adding contributors information. Feel free to give another pass before merging. None of the style errors seems to be related to this work. |
Co-authored-by: Santiago Soler <santisoler@fastmail.com>
lheagy
left a comment
lheagy
left a comment
There was a problem hiding this comment.
Thanks for your work on this @dccowan! I made a few wording suggestions to soften the language a bit as comments in the pr review. We can hold a high standard, but using words like "strict" aren't necessarily inviting, so I made a few minor suggestions. The pull request review process is meant to be a way for us to provide feedback, so that is where we can ensure that any new content meets our standards.
Also, there was one comment about what not to contribute "tutorials containing data-specific results, where choices in parameter values are not generally applicable", which isn't clear to me. If someone has an interesting dataset / result that they want to contribute, this statement might deter them. I would suggest we remove this and instead encourage folks to start an issue if they are interested but unsure if their content is a fit.
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
To ensure consistency, we should add a PR template for new notebooks. I have added a PR template. The checklist should ensure that all notebook requirements are met.