12
edits
DevTools/Code Review Checklist: Difference between revisions
DevTools/Code Review Checklist (view source)
Revision as of 13:33, 26 February 2018
, 26 February 2018Mark page for deletion after team discussion in https://github.com/devtools-html/rfcs/issues/37
(First draft) |
Jdescottes (talk | contribs) (Mark page for deletion after team discussion in https://github.com/devtools-html/rfcs/issues/37) |
||
| (3 intermediate revisions by 3 users not shown) | |||
| Line 1: | Line 1: | ||
{{delete|Moved to devtools docs (http://docs.firefox-dev.tools/contributing/code-reviews.html) |date=26 Feb 2018}} | |||
This checklist is primarily useful for DevTools reviewers as it lists all points potentially important to check while reviewing code, but it can serve as a set of guidelines for patch authors too. | This checklist is primarily useful for DevTools reviewers as it lists all points potentially important to check while reviewing code, but it can serve as a set of guidelines for patch authors too. | ||
| Line 7: | Line 9: | ||
* Commit message says what is being changed and why as described here : http://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview/commits.html#write-detailed-commit-messages | * Commit message says what is being changed and why as described here : http://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview/commits.html#write-detailed-commit-messages | ||
* Patch applies locally to current sources with no merge required | * Patch applies locally to current sources with no merge required | ||
* Check that every new file introduced by the patch has proper Mozilla license header : https://www.mozilla.org/en-US/MPL/headers/ | |||
= Manual testing = | = Manual testing = | ||
| Line 39: | Line 42: | ||
* User facing changes | * User facing changes | ||
** If a new piece of UI or new user interaction is added/modified, then Helen is ui-review? on the bug | ** If a new piece of UI or new user interaction is added/modified, then Helen is ui-review? on the bug | ||
** If a user facing string has been added, it is localized and follows the localization | ** If a user facing string has been added, it is localized and follows the localization guidelines at : https://wiki.mozilla.org/DevTools/Hacking#Guidelines | ||
** If a user-facing string has changed meaning, the key has been updated : https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings | ** If a user-facing string has changed meaning, the key has been updated : https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings | ||
** If a new image is added, it is a SVG image or there is a reason for not using a SVG | ** If a new image is added, it is a SVG image or there is a reason for not using a SVG | ||