Skip to content

Commit

Permalink
move reviewer guide and checklist to new file
Browse files Browse the repository at this point in the history
  • Loading branch information
sarah-hlsn committed Nov 1, 2023
1 parent cdffb6b commit 9c3cf45
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 72 deletions.
54 changes: 0 additions & 54 deletions doc/guide/Guide_Git_Development.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -608,60 +608,6 @@
"12) Update the `develop` branch on your local machine."
]
},
{
"cell_type": "markdown",
"metadata": {
"slideshow": {
"slide_type": "slide"
}
},
"source": [
"### How to review a pull request\n",
"\n",
"- Be friendly\n",
"- Read and follow the [**Reviewer Checklist**](../guide/Guide_Reviewer_Checklist.ipynb)\n",
"- Decide how much time you can spare and the detail you can work in. Tell the author!\n",
"- Use the comment/chat functionality within GitHub's pull requests - it's useful to have an archive of discussions and the decisions made.\n",
"- Fix the big things first! If there are more important issues, not every style guide has to be stuck to,\\\n",
" not every slight increase in speed needs to be pointed out, and test coverage doesn't have to be 100%.\n",
"- Make it clear when a change is optional, or is a matter of opinion"
]
},
{
"cell_type": "markdown",
"metadata": {
"slideshow": {
"slide_type": "subslide"
}
},
"source": [
"At a minimum\n",
"- Make sure unit and integration tests are passing.\n",
"- (For complete modules) Run the tutorial on your local machine and check it does what it says it does\n",
"- Check everything is fully documented"
]
},
{
"cell_type": "markdown",
"metadata": {
"slideshow": {
"slide_type": "subslide"
}
},
"source": [
"At least one reviewer needs to\n",
"- Review all the changes in the pull request. Read what it's supposed to do, check it does that, and make sure the logic is sound.\n",
"- Check that the code follows the CLIMADA style guidelines `#TODO: link`\n",
"- If the code is implementing an algorithm it should be referenced in the documentation. Check it's implemented correctly.\n",
"- Try to think of edge cases and ways the code could break. See if there's appropriate error handling in cases where the function might behave unexpectedly.\n",
"- (Optional) suggest easy ways to speed up the code, and more elegant ways to achieve the same goal.\n",
"\n",
"There are a few ways to suggest changes\n",
"- As questions and comments on the pull request page\n",
"- As code suggestions (max a few lines) in the code review tools on GitHub. The author can then approve and commit the changes from GitHub pull request page. This is great for typos and little stylistic changes.\n",
"- If you decide to help the author with changes, you can either push them to the same branch, or create a new branch and make a pull request with the changes back into the branch you're reviewing. This lets the author review it and merge."
]
},
{
"cell_type": "markdown",
"metadata": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,58 @@
"cells": [
{
"cell_type": "markdown",
"id": "commercial-participant",
"metadata": {},
"source": [
"# Reviewer Checklist\n",
"# Reviewer Guidelines"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"## How to review a pull request\n",
"\n",
"- Be friendly\n",
"- Read and follow the [**Reviewer Checklist**](../guide/Guide_Reviewer_Checklist.ipynb)\n",
"- Decide how much time you can spare and the detail you can work in. Tell the author!\n",
"- Use the comment/chat functionality within GitHub's pull requests - it's useful to have an archive of discussions and the decisions made.\n",
"- Fix the big things first! If there are more important issues, not every style guide has to be stuck to,\\\n",
" not every slight increase in speed needs to be pointed out, and test coverage doesn't have to be 100%.\n",
"- Make it clear when a change is optional, or is a matter of opinion"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"At a minimum\n",
"- Make sure unit and integration tests are passing.\n",
"- (For complete modules) Run the tutorial on your local machine and check it does what it says it does\n",
"- Check everything is fully documented"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"At least one reviewer needs to\n",
"- Review all the changes in the pull request. Read what it's supposed to do, check it does that, and make sure the logic is sound.\n",
"- Check that the code follows the CLIMADA style guidelines `#TODO: link`\n",
"- If the code is implementing an algorithm it should be referenced in the documentation. Check it's implemented correctly.\n",
"- Try to think of edge cases and ways the code could break. See if there's appropriate error handling in cases where the function might behave unexpectedly.\n",
"- (Optional) suggest easy ways to speed up the code, and more elegant ways to achieve the same goal.\n",
"\n",
"There are a few ways to suggest changes\n",
"- As questions and comments on the pull request page\n",
"- As code suggestions (max a few lines) in the code review tools on GitHub. The author can then approve and commit the changes from GitHub pull request page. This is great for typos and little stylistic changes.\n",
"- If you decide to help the author with changes, you can either push them to the same branch, or create a new branch and make a pull request with the changes back into the branch you're reviewing. This lets the author review it and merge."
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"## Reviewer Checklist\n",
"\n",
"- The code must be readable without extra effort from your part. The\n",
" code should be easily readable (for infos e.g.\n",
Expand Down Expand Up @@ -73,24 +121,10 @@
}
],
"metadata": {
"kernelspec": {
"display_name": "Python 3 (ipykernel)",
"language": "python",
"name": "python3"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.8.12"
"name": "python"
}
},
"nbformat": 4,
"nbformat_minor": 5
"nbformat_minor": 2
}

0 comments on commit 9c3cf45

Please sign in to comment.