1934
Comment:
|
← Revision 6 as of 2021-11-11 09:59:37 ⇥
1954
Move Python style guide to RTD
|
Deletions are marked like this. | Additions are marked like this. |
Line 2: | Line 2: |
'''This page needs to be cleaned up''' = Review instructions/policy/workflow = |
|
Line 3: | Line 5: |
= Review instructions/policy/workflow = | * Things you can do, as a reviewer, to avoid round trips * Include diff chunks for context * Do not make reference to line numbers, as diffs are a moving target * Always include the file name in diff chunks |
Line 9: | Line 14: |
* UserInterfaceChecklist - low-level Web design and e-mail design expectations | * [[../UI/Reviews|UIReviews]] - low-level Web design and e-mail design expectations |
Line 12: | Line 17: |
* PythonStyleGuide * JavaScriptStyleGuide |
* [[https://launchpad.readthedocs.io/en/latest/guides/python.html|Python style guide]] * JavaScriptReviewNotes |
Line 16: | Line 21: |
* MentorProcess - Special instructions for both mentors and recruits * LaunchpadProductionDocumentation - operational environment of our servers. Use this to understand what additional constraints on dev code might be needed (e.g. which machines can connect to the outside world, which need proxies, etc.) |
This page needs to be cleaned up
Review instructions/policy/workflow
- Things you can do, as a reviewer, to avoid round trips
- Include diff chunks for context
- Do not make reference to line numbers, as diffs are a moving target
- Always include the file name in diff chunks
- Please subscribe to these wiki pages (they are low volume, but changes matter):
PreMergeReviews - why and how we are doing this.
WorkingWithReviews - the workflow from the perspective of a coder wanting to land a branch - what you can expect from them, and what they are expecting from you.
LaunchpadHackingFAQ - the instructions here document the expectations we are setting for code that comes up for review - even if we have issues and questions not expressed there, all code that is oked should pass all of the criterion in that FAQ
UIReviews - low-level Web design and e-mail design expectations
CodeReviewChecklist - checklist that should be pass by all reviewed code
TipsForReviewers
ReviewerMeetingAgenda - when and where reviewers meet on irc.
- Changes to security.cfg can be made even during DB freeze. This runs the risk of causing oopses on edge.launchpad.net, if edge requires a permission that has not been granted to the production DB. So if you are landing a security.cfg change while the DB is frozen, you need to email stub, mthaddon, CC: launchpad@ asking them to apply the manual change to jubany as well. Reviewers should remind about this when seeing a security.cfg change.
Suggested reading
http://c2.com/cgi/wiki?CodeReviewPatterns has a wealth of information about how other people do code reviews, and it is fundamentally good stuff - have a read through at some point.