Confirmed users
978
edits
User:Hsinyi/Gecko engineering: Difference between revisions
→Initial communication between requesters & reviewers: commit messages
(→Code review: Initial communication between requesters & reviewers) |
(→Initial communication between requesters & reviewers: commit messages) |
||
| Line 83: | Line 83: | ||
*:[https://docs.google.com/document/d/1Tk-DY1aLTNtfRqrngAmLo8Eivqo9zohsCgAyNUBfVFQ/edit Lazy image loading architecture review] | *:[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] | *:[https://docs.google.com/document/d/1fOlyHR2R3qFi390fj_j33-LaPd36jzdbKHOF6w7SdNk/edit OOP-iframe event delivery in Fission] | ||
====Creating a patch & testing==== | |||
===== Commit messages ===== | |||
''“Changesets should have commit messages. The commit message should describe not just the "what" of the change but also the "why". This is especially true in cases when the "what" is obvious from the diff anyway; for larger changes it makes sense to have a summary of the "what" in the commit message.'' | |||
''As a specific example, if your diff is a one-line change that changes a method call argument from "true" to "false", having a commit message that says "change argument to mymethod from true to false" is not very helpful at all. A good commit message in this situation will at least mention the meaning for the argument. If that does not make it clear why the change is being made, the commit message should explain the "why".''<sub>[https://groups.google.com/g/mozilla.dev.platform/c/qERnoJniCds/m/YWmmKqe4EAAJ 2]</sub> | |||
*Describe WHY in addition to WHAT | |||
*Describe the change but not the symptom. Bad example: [Bug XYZ: Crash at Foo::Bar" ] | |||
*For WPTs which will be upstreamed, consider how the commit message will read outside the m-c repository, and at least mention which features the tests are intending to cover. | |||
**For example, a commit message like "Bug 1234 - Part 2: Tests" is not totally terrible in the context of Mozilla Central (though can be better) but "Part 2: Tests" is utterly meaningless upstream. | |||
*Reviewers should feel comfortable to reject a review request if the commit message isn’t well-written or doesn’t provide useful information. | |||
*[[https://groups.google.com/g/mozilla.dev.platform/c/qERnoJniCds/m/6DLNfrGhBQAJ Open question]] People have various preferences on the length of commit messages … We need more discussions and a much better style guide, explaining what makes a good commit message and what makes a good and descriptive bug, with a number of (good and bad) examples. | |||
== Release health and product quality == | == Release health and product quality == | ||
== Roadmap and status communication == | == Roadmap and status communication == | ||