Expand code reviews section #42
Conversation
|
I would recommend removing "Prefer small reviews to large ones" as this is now covered in greater depth in the text added in this PR. I have not already made this change as that is text I did not originally author. |
|
My initial idea was to keep the "general principles" pages short and very high-level and detail ideas in separate pages. But I'm now unsure of what's the best approach 🤔. @avallecam @annacarnegie, do you have any recommendations in terms of format? |
|
Grate addition on code reviews, @joshwlambert. @Bisaloo I agree with the idea to keep the "general" short and extend on detailed points on a separate page, similar to the "standards for branching". Also, add a hyperlink to that new page on the box "Read more about this principle in application" |
|
If everyone agrees, I'll move the information I've added to a new page within this PR. |
|
@Bisaloo @avallecam the text is now in a file named code-review.qmd. It is linked in the document index. Happy for either of you to approve or directly merge the changes if your happy with the update or just let me know if there are other changes required before merging. |
|
It looks good to me, thanks for working on it! |
There was a problem hiding this comment.
Sounds very good to me! Thanks for contributing! This is ready to go as far as I'm concerned.
Next item on this page would be to address Seb's question in #21: who merges the PR? The author or the reviewer? My personal preference is reviewers approve and author merges but we could present this question to the group.
This PR addresses #41.
It extends the Code reviews section of blueprints to include reference to the Tidyteam code review principles, adds explicit points of similarity between Tidyteam and Epiverse-TRACE review practises, and some extra information to aid both Epiverse-TRACE developers or community members how to engage in code reviews.
The text introduced in this PR has caused the Code reviews section to become the largest in the General principles page. I do not have a strong preference as to whether we keep this new information in it's current section or more the new text to a new page in blueprints. This new page might be a good catalyst to work on Policies around Issues/PRs.