1825821 - Add front end team's Code Review Standards to docs

Status: RESOLVED FIXED

(Developer Infrastructure :: Firefox Source Docs: Content, task, P3)

Description

[Sarah Clements [:sclements]]

The frontend desktop team has its own document on code review standards that should be added to the firefox source docs.

Comment 1

[Tim Giles [:tgiles]]

To help Mozilla out with this bug, here's the steps:

1. Comment here on the bug that you want to volunteer to help.
   This will tell others that you're working on the next steps.
2. Download and build the Firefox source code
   • Here is a more detailed tutorial for getting set up to work on Firefox
   • If you have any problems, please ask on Element/Matrix in the #introduction channel. They're there to help you get started.
3. Start working on this bug.
   • First, run ./mach doc browser/docs. This will install the necessary dependencies and build only the browser section of the docs (there are lot of unnecessary docs that will be built if you run just ./mach doc!).
     • This operation may take a few minutes!
     • There may be a lot of red/error text about "WARNING: document isn't included in any toctree", you can ignore this for the most part.
   • After running ./mach docs browser/docs, a browser should appear with the url being localhost:5500. This is the built documentation.
   • Create a new file called "CodeReviewBestPractices.md" under browser/docs/ and add this Firefox Desktop Front end Code Review Best Practices document to the newly created file and make sure it is formatted properly (including headings at the correct level).
   • You will also need to add "CodeReviewBestPractices" to the browser/docs/index.rst file to see your changes show up in the built documentation.
   • If you have any problems with this bug, please comment on this bug and set the needinfo flag for me.
4. Build your change with mach doc and verify your change by navigating to the newly created doc. Also check your changes for adherence to our style guidelines by using mach lint.
5. Submit the patch (including an automated test, if applicable) for review. Mark me as a reviewer so I'll get an email to come look at your code.
   • How to Submit a Patch
   • This is when the bug will be assigned to you.
6. After a series of reviews and changes to your patch, I'll push it to autoland.
   • If there are changes requested, please read the "To update a submitted patch" section to ensure you don't accidentally create a duplicate revision!
   • Your code will soon be shipping to Firefox users worldwide!

Comment 2

[Stuart Colville [:muffinresearch]]

Attachment: Bug 1825821 - Add code review best practices to source docs. r?tgiles

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.

Comment 4

[Stuart Colville [:muffinresearch]]

Attachment: Bug 1825821 - Add frontend code review best practices to source docs. r=mossop

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38733a0c76ae
Add frontend code review best practices to source docs. r=mossop

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/38733a0c76ae

Comment 7

[Sylvestre Ledru [:Sylvestre]]

It is now available:
https://firefox-source-docs.mozilla.org/browser/FrontendCodeReviewBestPractices.html

But we had a few docs about this:
https://firefox-source-docs.mozilla.org/contributing/reviews.html
https://firefox-source-docs.mozilla.org/contributing/Code_Review_FAQ.html
https://firefox-source-docs.mozilla.org/contributing/reviewer_checklist.html
https://firefox-source-docs.mozilla.org/contributing/how_to_submit_a_patch.html
https://firefox-source-docs.mozilla.org/devtools/contributing/code-reviews.html
https://firefox-source-docs.mozilla.org/devtools/contributing/code-reviews-checklist.html
https://firefox-source-docs.mozilla.org/devtools/contributing/making-prs.html
https://firefox-source-docs.mozilla.org/l10n/fluent/review.html

As most of what is in https://firefox-source-docs.mozilla.org/browser/FrontendCodeReviewBestPractices.html isn't Frontend specific and probably documented elsewhere, it would be nice to invest more in a common review strategy instead of having one per org/team/project.

What do you think folks?

Comment 8

[Dave Townsend [:mossop]]

(In reply to Sylvestre Ledru [:Sylvestre] from comment #7)

It is now available:
https://firefox-source-docs.mozilla.org/browser/FrontendCodeReviewBestPractices.html

But we had a few docs about this:
https://firefox-source-docs.mozilla.org/contributing/reviews.html
https://firefox-source-docs.mozilla.org/contributing/Code_Review_FAQ.html
https://firefox-source-docs.mozilla.org/contributing/reviewer_checklist.html
https://firefox-source-docs.mozilla.org/contributing/how_to_submit_a_patch.html
https://firefox-source-docs.mozilla.org/devtools/contributing/code-reviews.html
https://firefox-source-docs.mozilla.org/devtools/contributing/code-reviews-checklist.html
https://firefox-source-docs.mozilla.org/devtools/contributing/making-prs.html
https://firefox-source-docs.mozilla.org/l10n/fluent/review.html

As most of what is in https://firefox-source-docs.mozilla.org/browser/FrontendCodeReviewBestPractices.html isn't Frontend specific and probably documented elsewhere, it would be nice to invest more in a common review strategy instead of having one per org/team/project.

What do you think folks?

It would be nice yes. I'm not sure who has the time to own that though.

Comment 9

[Stuart Colville [:muffinresearch]]

I think there's certainly a potential of perfection being the enemy of the good, and whilst additional consolidation could make sense in slower time, I'd be the first to acknowledge these efforts stalled previously due to not having time to prioritize making the previous attempt more universal. As a team our more immediate need was having something we can point to that covers what's needed specific to frontend even if there are overlaps with other existing docs.