Refine code review based on Kelli and Kathryn's workshop training

NOAA-FIMS / collaborative_workflow

contributors guide to FIMS, managing collaborations

https://noaa-fims.github.io/collaborative_workflow/

4 stars 1 forks source link

Refine code review based on Kelli and Kathryn's workshop training #74

Closed iantaylor-NOAA closed 2 years ago

iantaylor-NOAA commented 2 years ago

A few changes to this section that we should make to align this section with the ongoing training from @kellijohnson-NOAA and @k-doering-NOAA: https://noaa-fims.github.io/collaborative_workflow/contributor-guidelines.html#code-review

[x] revise the Review Checklist section to focus on new github action for PR checklists
[x] provide more detail on automated tests
- [x] clarify that developer should write tests prior to the PR
- [x] make clear that they will be run automatically for each PR
- [x] note that the results of the tests should be shown on the GitHub PR page

Please edit this issue to add more checklist items as appropriate.

k-doering-NOAA commented 2 years ago

Thanks @iantaylor-NOAA ! Some revisions are in this PR that hasn't been merged yet (people can add to this if they want to make changes): https://github.com/NOAA-FIMS/collaborative_workflow/pull/73

kristanblackhart-NOAA commented 2 years ago

@iantaylor-NOAA The items you note related to testing are described in the testing section of the workflow document. Do we need to make edits in the review section to specify these details?

iantaylor-NOAA commented 2 years ago

Thanks @k-doering-NOAA and @kristanblackhart-NOAA. I just looked through PR #73 and the brief description of the checklist seems fine, so I think that covers the first item on the checklist.

I also looked at https://noaa-fims.github.io/collaborative_workflow/testing.html#general-test-case-documentation which covers the remaining points. In skimming the testing section before writing this issue, I incorrectly though the tables were all specific examples, but the general overview provided in the first table is great.

If someone in the morning training session sees additional changes to the code review section that are needed, please feel free to re-open this issue.