Feature/trainee code review guide#1773
Feature/trainee code review guide#1773chinar-amrutkar wants to merge 6 commits intoCodeYourFuture:mainfrom
Conversation
New module covering: - Why code review matters - What to look for when reviewing code - How to give constructive feedback - Specific guidance on reviewing AI-generated code Addresses issue CodeYourFuture#1651: Introduce trainees to reviewing code
✅ Deploy Preview for cyf-curriculum ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
👷 Deploy request for cyf-piscine pending review.Visit the deploys page to approve it
|
✅ Deploy Preview for cyf-sdc canceled.
|
👷 Deploy request for cyf-tracks pending review.Visit the deploys page to approve it
|
👷 Deploy request for cyf-itd pending review.Visit the deploys page to approve it
|
👷 Deploy request for cyf-launch pending review.Visit the deploys page to approve it
|
👷 Deploy request for cyf-common pending review.Visit the deploys page to approve it
|
illicitonion
requested review from
LonMcGregor
and removed request for
illicitonion
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good work!
Do you think it would be worth adding links somewhere near the end to the more detailed guidance we have, as an "optional extra" people could look at if they are interested in further reading?
- Our style guide, which serves as a good example of what trainees might encounter in the workplace: https://curriculum.codeyourfuture.io/guides/reviewing/style-guide/
- the "what to look for" page which goes into more detail about specific problems that might be found during a review: https://curriculum.codeyourfuture.io/guides/reviewing/checklist/
Also, is this just a module at the moment, or is it going to be included in a specific place within the curriculum? I can't tell just from the PR. I ask because the link at the bottom points to the SDC slack - are we definitely intending this only to be used by SDC trainees? if ITP trainees could see this we might need a different link.
Added links to CYF style guide and code review checklist as optional further reading resources, as suggested in review feedback.
|
Thanks for the review! Added the further reading links. Good call on including the style guide and checklist as optional extras. To clarify the scope: this is intended for SDC trainees only, so the #cyf-code-review Slack link stays as-is. If ITP trainees end up using it in the future we can add a note about which channel to use. |
|
Thanks for changing that, we've now moved all the slack links to now point to the community slack group, including for SDC as well. So you would need to update the link. I've enabled the netlify preview for this, can you visit it above and check it all displays correctly? (also, I cant find the page URL, which page will this end up being accessible at?) |
2 participants
For issue #1651.