tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

reviews.rst (13050B)


      1 Getting reviews
      2 ===============
      3 
      4 
      5 Thorough code reviews are one of Mozilla's ways of ensuring code quality.
      6 Every patch must be reviewed by the module owner of the code, or one of their designated peers.
      7 
      8 Commit message syntax
      9 ---------------------
     10 
     11 When submitting your commit(s), use the following syntax to request review for your patch:
     12 
     13 .. list-table::
     14   :header-rows: 1
     15 
     16   * - Request
     17     - Syntax
     18     - Description
     19   * - Single reviewer
     20     - ``r=reviewer``
     21     - Request a review from a single reviewer (``reviewer``). Historically, the syntax ``r?reviewer`` requested a review, and ``r=reviewer`` marked the reviewer who accepted the patch before landing. Both can now be used interchangeably when requesting a review.
     22   * - Multiple reviewers
     23     - ``r=reviewer1,reviewer2``
     24     - Request reviews from *both* ``reviewer1`` and ``reviewer2``.
     25   * - Blocking review
     26     - ``r=reviewer!``
     27     - Request a *blocking* review from ``reviewer``. This means that ``reviewer`` *must* review the patch before it can be landed.
     28   * - Both optional and blocking reviews
     29     - ``r=reviewer1,reviewer2!``
     30     - Request a *blocking* review from ``reviewer2``, and an *optional* review from ``reviewer1``.
     31   * - Review groups
     32     - ``r=#review-group``
     33     - Request a review from members of ``#review-group``. A full list can be found below.
     34   * - Blocking review groups
     35     - ``r=#review-group!``
     36     - Request a blocking review from a review group. This will require *at least one member* of the group to approve before landing.
     37 
     38 For example, the commit syntax to request review from group ``group-name`` or ``developer-nickname`` would be:
     39 
     40 .. code-block::
     41 
     42     Bug xxxx - explain what you are doing and why r?#group-name
     43 
     44     or
     45 
     46     Bug xxxx - explain what you are doing and why r?developer-nickname
     47 
     48 Reviews, and review groups, can be selected or adjusted after submission in the phabricator UI.
     49 
     50 Sometimes when publishing a patch, groups will automatically be added as blocking reviewers due to the code being touched. In this case, you may want to check that the reviewers you requested review from are set to blocking as well.
     51 
     52 Choosing reviewers
     53 ------------------
     54 
     55 * If you have a mentor assigned on the bug you are fixing, the mentor can usually either also review or find a suitable reviewer on your behalf.
     56 * If you do not have a mentor, see if your code fits into one of the review groups below, and request review from that group.
     57 * Otherwise, try looking at the history of the file to see who has modified it recently. (For example, `hg log <modified-file>` or `git log <modified-file>`)
     58 * Finally if you are still unable to identify someone, try asking in the `#introduction channel on Matrix <https://chat.mozilla.org/#/room/#introduction:mozilla.org>`_.
     59 
     60 
     61 Getting attention
     62 -----------------
     63 
     64 Generally most reviews will happen within roughly a week. If, however, a reviewer doesn't respond within a week or so of the review request:
     65 
     66  * Contact the reviewer directly (either via e-mail or on Matrix).
     67  * Join developers on `Mozilla's Matrix server <https://chat.mozilla.org>`_, and ask if anyone knows why a review may be delayed. Please link to the bug too.
     68  * If the review is still not addressed, mail the reviewer directly, asking if/when they'll have time to review the patch, or might otherwise be able to review it.
     69 
     70 Remember that reviewers are human too, and may have complex reasons that prevent them from reviewing your patch in a timely manner. Be confident in reaching out to your reviewer, but be mindful of the `Mozilla Community Participation Guidelines <https://www.mozilla.org/en-US/about/governance/policies/participation/>`_ while doing so.
     71 
     72 For simple documentation changes, reviews are not required.
     73 
     74 For more information about the review process, see the :ref:`Code Review FAQ`.
     75 
     76 Review groups
     77 -------------
     78 
     79 
     80 .. list-table::
     81   :header-rows: 1
     82 
     83   * - Name
     84     - Owns
     85     - Members
     86   * - #anchor-positioning-reviewers
     87     - Anchor positioning - related style and layout code
     88     - `Member list <https://phabricator.services.mozilla.com/project/members/216/>`__
     89   * - #anti-tracking
     90     - `Core: Anti-Tracking </mots/index.html#core-anti-tracking>`__
     91     - `Member list <https://phabricator.services.mozilla.com/project/members/157/>`__
     92   * - #build or #firefox-build-system-reviewers
     93     - The configure & build system
     94     - `Member list <https://phabricator.services.mozilla.com/project/members/20/>`__
     95   * - #cookies
     96     - `Core: Cookies </mots/index.html#core-cookies>`__
     97     - `Member list <https://phabricator.services.mozilla.com/project/members/177/>`__
     98   * - #cubeb-reviewers
     99     - cubeb, Gecko's audio input/output library and associated projects (audioipc, cubeb-rs, rust cubeb backends)
    100     - `Member list <https://phabricator.services.mozilla.com/project/profile/129/>`__
    101   * - #desktop-theme-reviewers
    102     - `User interface CSS </mots/index.html#desktop-theme>`__
    103     - `Member list <https://phabricator.services.mozilla.com/project/members/141/>`__
    104   * - #devtools-reviewers
    105     - `Firefox DevTools </mots/index.html#devtools>`__
    106     - `Member list <https://phabricator.services.mozilla.com/project/members/153/>`__
    107   * - #dom-core
    108     - `Core: DOM <https://firefox-source-docs.mozilla.org/mots/index.html#core-document-object-model>`__
    109     - `Member list <https://phabricator.services.mozilla.com/project/members/178/>`__
    110   * - #dom-worker-reviewers
    111     - DOM Workers
    112     - `Member list <https://phabricator.services.mozilla.com/project/members/146/>`__
    113   * - #dom-storage-reviewers
    114     - DOM Storage
    115     - `Member list <https://phabricator.services.mozilla.com/project/members/147/>`__
    116   * - #extension-reviewers
    117     - `WebExtensions </mots/index.html#webextensions>`__ and `Toolkit::Add-ons Manager </mots/index.html#add-ons-manager>`__
    118     - `Member list <https://phabricator.services.mozilla.com/project/members/130/>`__
    119   * - #fluent-reviewers
    120     - Fluent (FTL) files (translation).
    121     - `Member list <https://phabricator.services.mozilla.com/project/members/105/>`__
    122   * - #firefox-source-docs-reviewers
    123     - Documentation files and its build
    124     - `Member list <https://phabricator.services.mozilla.com/project/members/118/>`__
    125   * - #firefox-ux-team
    126     - User experience (UX)
    127     - `Member list <https://phabricator.services.mozilla.com/project/members/91/>`__
    128   * - #firefox-svg-reviewers
    129     - SVG-related changes
    130     - `Member list <https://phabricator.services.mozilla.com/project/members/97/>`__
    131   * - #frontend-codestyle-reviewers
    132     - ESLint, Prettier or Stylelint configurations.
    133     - `Member list <https://phabricator.services.mozilla.com/project/members/208/>`__
    134   * - #android-reviewers
    135     - Fenix, Focus and Android Components.
    136     - `Member list <https://phabricator.services.mozilla.com/project/members/200/>`__
    137   * - #geckoview-reviewers
    138     - GeckoView
    139     - `Member list <https://phabricator.services.mozilla.com/project/members/92/>`__
    140   * - #gfx-reviewers
    141     - Graphics code
    142     - `Member list <https://phabricator.services.mozilla.com/project/members/122/>`__
    143   * - #intermittent-reviewers
    144     - Test manifest changes
    145     - `Member list <https://phabricator.services.mozilla.com/project/members/110/>`__
    146   * - #ipc-reviewers
    147     - `Core: IPC <https://firefox-source-docs.mozilla.org/mots/index.html#core-ipc>`__
    148     - `Member list <https://phabricator.services.mozilla.com/project/members/145/>`__
    149   * - #layout-reviewers
    150     - Layout
    151     - `Member list <https://phabricator.services.mozilla.com/project/members/126/>`__
    152   * - #layout-grid-reviewers
    153     - layout/grid
    154     - `Member list <https://phabricator.services.mozilla.com/project/members/215/>`__
    155   * - #linter-reviewers
    156     - tools/lint/*
    157     - `Member list <https://phabricator.services.mozilla.com/project/members/119/>`__
    158   * - #mac-reviewers
    159     - Mac-specific code
    160     - `Member list <https://phabricator.services.mozilla.com/project/members/149/>`__
    161   * - #media-playback-reviewers
    162     - `Media playback <https://wiki.mozilla.org/Modules/All#Media_Playback>`__
    163     - `Member list <https://phabricator.services.mozilla.com/project/profile/159/>`__
    164   * - #mozbase
    165     - Mozbase
    166     - `Member list <https://phabricator.services.mozilla.com/project/members/113/>`__
    167   * - #mozbase-rust
    168     - Mozbase in Rust
    169     - `Member list <https://phabricator.services.mozilla.com/project/members/114/>`__
    170   * - #necko-reviewers
    171     - network code (aka necko, aka netwerk)
    172     - `Member list <https://phabricator.services.mozilla.com/project/members/127/>`__
    173   * - #nss-reviewers
    174     - Network Security Services (NSS)
    175     - `Member list <https://phabricator.services.mozilla.com/project/members/156/>`__
    176   * - #perftest-reviewers
    177     - Perf Tests
    178     - `Member list <https://phabricator.services.mozilla.com/project/members/102/>`__
    179   * - #permissions or #permissions-reviewers
    180     - `Permissions </mots/index.html#core-permissions>`__
    181     - `Member list <https://phabricator.services.mozilla.com/project/members/158/>`__
    182   * - #places-reviewers
    183     - `Bookmarks & History (Places) </mots/index.html#bookmarks-history>`__
    184     - `Member list <https://phabricator.services.mozilla.com/project/members/186/>`__
    185   * - #platform-i18n-reviewers
    186     - Platform Internationalization
    187     - `Member list <https://phabricator.services.mozilla.com/project/members/150/>`__
    188   * - #preferences-reviewers
    189     - Firefox for Desktop Preferences (Options) user interface
    190     - `Member list <https://phabricator.services.mozilla.com/project/members/132/>`__
    191   * - #recomp-reviewers or #reusable-components-reviewers
    192     - UI Widgets, Design Tokens, Storybook
    193     - `Member list <https://phabricator.services.mozilla.com/project/members/185/>`__
    194   * - #remote-debugging-reviewers
    195     - Remote Debugging UI & tools
    196     - `Member list <https://phabricator.services.mozilla.com/project/members/108/>`__
    197   * - #search-reviewers
    198     - `Search </mots/index.html#search>`__
    199     - `Member list <https://phabricator.services.mozilla.com/project/members/169/>`__
    200   * - #sessionstore or #sessionstore-reviewers
    201     - `Firefox: Session Restore </mots/index.html#session-restore>`__
    202     - `Member list <https://phabricator.services.mozilla.com/project/members/179/>`__
    203   * - #spidermonkey-reviewers
    204     - SpiderMonkey JS/Wasm Engine
    205     - `Member list <https://phabricator.services.mozilla.com/project/members/173/>`__
    206   * - #static-analysis-reviewers
    207     - Static Analysis
    208     - `Member list <https://phabricator.services.mozilla.com/project/members/120/>`__
    209   * - #style or #firefox-style-system-reviewers
    210     - Firefox style system (servo, layout/style).
    211     - `Member list <https://phabricator.services.mozilla.com/project/members/90/>`__
    212   * - #supply-chain-reviewers
    213     - Third-party audits and vendoring (cargo-vet, supply_chain).
    214     - `Member list <https://phabricator.services.mozilla.com/project/members/164/>`__
    215   * - #tabbrowser or #tabbrowser-reviewers
    216     - `Firefox: Tabbed Browser </mots/index.html#tabbed-browser>`__
    217     - `Member list <https://phabricator.services.mozilla.com/project/members/180/>`__
    218   * - #theme or #desktop-theme-reviewers
    219     - `Firefox: Theme and Toolkit: Themes </mots/index.html#desktop-theme>`__
    220     - `Member list <https://phabricator.services.mozilla.com/project/members/141/>`__
    221   * - #translations-reviewers
    222     - `Firefox: Translation <https://firefox-source-docs.mozilla.org/mots/index.html#translation>`__
    223     - `Member list <https://phabricator.services.mozilla.com/project/members/192/>`__
    224   * - #urlbar-reviewers
    225     - `Urlbar (Address Bar) </mots/index.html#address-bar>`__
    226     - `Member list <https://phabricator.services.mozilla.com/project/members/211/>`__
    227   * - #view-transitions-reviewers or #view-transitions or #vt
    228     - View Transitions (dom/view-transitions, and the relevant style / layout / gfx code).
    229     - `Member list <https://phabricator.services.mozilla.com/project/members/217/>`__
    230   * - #webcompat-reviewers
    231     - System addons maintained by the Web Compatibility team
    232     - `Member list <https://phabricator.services.mozilla.com/project/members/124/>`__
    233   * - #webdriver-reviewers
    234     - Marionette and geckodriver (including MozBase Rust), and Remote Protocol with WebDriver BiDi, and CDP.
    235     - `Member list <https://phabricator.services.mozilla.com/project/members/103/>`__
    236   * - #webgpu-reviewers
    237     - WebGPU code
    238     - `Member list <https://phabricator.services.mozilla.com/project/members/170/>`__
    239   * - #webidl
    240     - WebIDL
    241     - `Member list <https://phabricator.services.mozilla.com/project/members/112/>`__
    242   * - #xpcom-reviewers
    243     - XPCOM
    244     - `Member list <https://phabricator.services.mozilla.com/project/members/125/>`__
    245 
    246 To create a new group, fill a `new bug in Conduit::Administration <https://bugzilla.mozilla.org/enter_bug.cgi?product=Conduit&component=Administration>`__.
    247 See `bug 1613306 <https://bugzilla.mozilla.org/show_bug.cgi?id=1613306>`__ as example.