Confirmed users
978
edits
User:Hsinyi/Gecko engineering: Difference between revisions
→Code review: Initial communication between requesters & reviewers
(→Code review: purpose of code review) |
(→Code review: Initial communication between requesters & reviewers) |
||
| Line 61: | Line 61: | ||
Additionally, knowledge sharing is another major advantage. | Additionally, knowledge sharing is another major advantage. | ||
===What does the code review process look like=== | |||
A critical thing to remember is that coding to some extent is a social process. The code review process may start '''before''' an actual patch and actual review request are submitted. '''Communication''' is key. It is very important and helpful that before you start implementing you discuss your implementation plan with the module peers, especially when you are working on something new to you or you are working on a new feature. Because documentation can be lacking or out of date, talking to the other members may save you literally weeks of needless time. It’s also because walking reviewers through your thoughts can help save their time and allow them to provide early feedback, too. | |||
With this thought in mind, the code review process actually contains the [https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch steps] in the red rectangular. | |||
[[File:(Draft) Gecko Platform Code Review Best Practice.png|thumb]] | |||
====Initial communication between requesters & reviewers ==== | |||
*Mutual understanding of the priority and the expected timeline | |||
*:As a requester, set a clear expectation. For a new feature or big patch, it’s especially important to plan in advance. | |||
*:As a reviewer, set a clear expectation about your availability, too. | |||
*Check reviewer’s availability before/when submitting a patch to avoid needless waiting time | |||
*:Reviewers should ensure that their phabricator calendars are up-to-date so that requesters will receive warnings when requests are submitted to a reviewer who is not available. | |||
*:Reviewers should ensure their bugzilla user name displays the Out-of-office information, if they are away for more than 2 days. | |||
*:Reviewers should consider to block accepting bugzilla needinfo/review requests, if they are away for more than 2 days. | |||
*Architecture design documents help! Examples: | |||
*:[https://docs.google.com/document/d/1Tk-DY1aLTNtfRqrngAmLo8Eivqo9zohsCgAyNUBfVFQ/edit Lazy image loading architecture review] | |||
*:[https://docs.google.com/document/d/1fOlyHR2R3qFi390fj_j33-LaPd36jzdbKHOF6w7SdNk/edit OOP-iframe event delivery in Fission] | |||
== Release health and product quality == | == Release health and product quality == | ||
== Roadmap and status communication == | == Roadmap and status communication == | ||