Per discussion this morning with @dcamron @brian-rose and @jnmorley
We mention reviews and waiting for reviews before merging a PR, but we should document some tips on what to look for when reviewing a PR.
Some thoughts from the conversation:
Look at the description and linked GitHub issue, you want to know if the PR addresses the issue
Look at the files changed to see where the changes occurred
Provide feedback on the code itself (are there tests, can you follow it, are variable names conflicting etc)
Look at the content for spelling errors
Visit the Previewed Site for image resolution and functionality (this won't tell you where the changes were, and be careful because some links will take you to the site itself and not the preview)
Notebook files can be reviewed with ReviewNB (GitHub doesn't render them super well and the code is mucked up)
Voice the limitations of your review! It is okay to not address all of these points in your review, but let us know what you did look for (content vs code, e.g.). Your opinion matters.
This could make a good blog post! I might right that up before creating a more formal document.
Per discussion this morning with @dcamron @brian-rose and @jnmorley
We mention reviews and waiting for reviews before merging a PR, but we should document some tips on what to look for when reviewing a PR.
Some thoughts from the conversation:
This could make a good blog post! I might right that up before creating a more formal document.