Bug 1596342 - Update sending patch documentation to only mention Phabricator + moz-phab. r=rcaliman.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 14 Nov 2019 16:49:44 +0000
changeset 501989 ee728f3ea085900757752d009ea271d21087096a
parent 501988 738801392270a0162d381b804fdb7f8111a57a42
child 501990 4a44d69250edf0378fe40d3b88dad9607cb54f78
child 502037 0fb79a3edf1bddd8532e6d98e7b0531b5155a6e4
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrcaliman
bugs1596342
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1596342 - Update sending patch documentation to only mention Phabricator + moz-phab. r=rcaliman. The documentation was mentioning arc as the primary tool to push to review (and was discouraging using moz-phab). moz-phab is now much more stable and is the prefered way of pushing patches, so we shouldn't mention arc anymore. There was also a (not recommended) mention of attaching patch to Bugzilla for reviews, which I think isn't something we recommend anymore. Differential Revision: https://phabricator.services.mozilla.com/D52969
devtools/docs/contributing/code-reviews-setup.md
devtools/docs/contributing/making-prs.md
--- a/devtools/docs/contributing/code-reviews-setup.md
+++ b/devtools/docs/contributing/code-reviews-setup.md
@@ -19,16 +19,9 @@ If you added an `:ircnickname` in your B
 ---
 
 Once you understand the above, please [create a Phabricator account](https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#creating-an-account). 
 
 
 
 ## Set up to send code for review
 
-There are two ways of doing this (sorry, let us explain!):
-
-* you can use [Arcanist](https://moz-conduit.readthedocs.io/en/latest/arcanist-user.html), which is the official command line tool that accompanies Phabricator, and should be enough for most of the cases.
-* or you could use [moz-phab](https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#using-moz-phab), which is a Mozilla-developed wrapper for Arcanist that makes it work better with the "Mozilla workflow".
-
-**We recommend you use Arcanist** for now, unless you are more experienced and know what you're doing, or want to take advantage of `moz-phab`'s features. You need to install Arcanist for `moz-phab` to work anyway.
-
-If you decide to use `moz-phab`, please be aware that we started using this new tool quite recently, and you might find bugs (or things that don't feel quite right), in which case you could either [have a look at the existing bugs](https://bugzilla.mozilla.org/buglist.cgi?product=Conduit&component=Review%20Wrapper&resolution=---) to see if someone else has encountered this again, or simply [file a bug](https://bugzilla.mozilla.org/enter_bug.cgi?product=Conduit&component=Review%20Wrapper) using your fancy new Bugzilla account 😀
+In order to push your commit to Phabricator, you need to install [moz-phab](https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#using-moz-phab).
--- a/devtools/docs/contributing/making-prs.md
+++ b/devtools/docs/contributing/making-prs.md
@@ -1,95 +1,66 @@
 # Sending your code for review (also known as "sending patches")
 
-There are two ways of doing this. We'll first explain the recommended version, and then the alternative, which is older, and we don't recommend, but might be handy in some circumstances.
-
-## Using Phabricator + Differential (RECOMMENDED)
-
 First, commit your changes. For example:
 
 ```bash
 hg add /path/to/file/changed
 hg commit -m "Bug 1234567 - Implement feature XYZ. r=name,name2!"
 ```
 
  The commit message explained in detail:
  - `Bug 1234567` - The number of the bug in bugzilla.
  - `- Implement feature XYZ.` - The commit message.
- - `r=name` - The short form to request a review.
- - `,name2!` - You can have more than one reviewer. The `!` makes the review a *blocking* review (Patch can not land without accepted review). You will also need to add it to the name in the popup after you ran `arc diff`.
+ - `r=name` - The short form to request a review. Enter the name you found using the 
+ instructions in the [previous step](./code-reviews-find-reviewer.md).
+ - `,name2!` - You can have more than one reviewer. The `!` makes the review a *blocking* review (Patch can not land without accepted review).
 
- Please note your first commit message will be also the title of your patch in Phabricator. (Don't worry, if something goes wrong. You can still change things in the UI later).
-
-Then create a revision in Differential, using Arcanist (or `moz-phab`):
+Then create a revision in Phabricator using `moz-phab`:
 
 ```bash
-arc diff
+moz-phab submit
 ```
 
-You'll be taken to an editor to add extra details, although some of them might be prepopulated using the data in the commit message. Make sure the bug number is filled with the right value. You'll also be able to set reviewers here: enter the names you found using the instructions in the [previous step](./code-reviews-find-reviewer.md).
-
-Save the changes and quit the editor. A revision will be created including those data and also the difference in code between your changes and the point in the repository where you based your work (this difference is sometimes called "a patch", as it's what you'd need to apply on the repository to get to the final state).
+A revision will be created including that information and the difference in code between your changes and the point in the repository where you based your work (this difference is sometimes called "a patch", as it's what you'd need to apply on the repository to get to the final state).
 
 If you click on the provided URL for the revision, it'll bring you to Phabricator's interface, which the reviewer will visit as well in order to review the code. They will look at your changes and decide if they need any additional work, based on whether the changes do fix what the bug describes or not. To get a better idea of the types of things they will look at and verify, read the [code reviews checklist](./code-reviews-checklist.md).
 
+For more information on using moz-phab, you can run:
+
+```bash
+moz-phab -h
+```
+
+or to get information on a specific command (here `submit`):
+
+```bash
+moz-phab submit -h
+```
+
 The reviewer might suggest you do additional changes. For example, they might recommend using a helper that already exists (but you were not aware of), or they might recommend renaming things to make things clearer. Or they might recommend you do *less* things (e.g. if you changed other things that are out of scope for the bug). Or they might simply ask questions if things aren't clear. You can also ask questions if the comments are unclear or if you're unsure about parts of the code you're interacting with. Something that looks very obvious to one person might confuse others.
 
-Hence, you might need to go back to the code and do some edits to address the issues and recommendations. Once you have done this, we want to amend the existing commit:
+Hence, you might need to go back to the code and do some edits to address the issues and recommendations. Once you have done this, you must update the existing commit:
 
 ```bash
 hg commit --amend -m 'Address revision issues: rewrite to use helper helpfulHelper()'
 ```
 
 And submit the change again:
 
 ```bash
-arc diff
-```
-
-This time, the editor that opens should have filled most of the information already. Add any missing information, save and quit the editor.
-
-You might have to go through this cycle of uploading a diff and getting it reviewed several times, depending on the complexity of the bug.
-
-**NOTE**: by default, Arcanist opens nano, which can be a bit confusing if you're not a nano user. You can override this by setting the `EDITOR` env variable. For instance, you could add this to your `.bash_profile` to set Vim as your preferred editor:
-
-```bash
-export EDITOR="/usr/local/bin/vim"
+moz-phab submit
 ```
 
-Once your code fixes the bug, and there are no more blocking issues, the reviewer will approve the review, and the code can be landed in the repository now.
-
-For more information on using Arcanist, please refer to [their user guide](https://moz-conduit.readthedocs.io/en/latest/arcanist-user.html#submitting-and-updating-patches).
-
-## Using Mercurial (not recommended)
-
-We don't recommend to use this method as it's more manual and makes reviewing and landing code a bit more tedious.
-
-### Creating a patch
-
-To create a patch you need to first commit your changes and then export them to a patch file.
+You might have to go through this cycle of submitting changes and getting it reviewed several times, depending on the complexity of the bug.
 
-```
-hg commit -m 'your commit message'
-hg export > /path/to/your/patch
-```
-
-### Commit messages
-
-Commit messages should follow the pattern `Bug 1234567 - change description. r=reviewer`.
+Once your code fixes the bug, and there are no more blocking issues, the reviewer will approve the changes, and the code can be landed in the repository now.
 
-First is the bug number related to the patch. Then the description should explain what the patch changes. The last part is used to keep track of the reviewer for the patch.
 
-### Submitting a patch
-
-Once you have a patch file, add it as an attachment to the Bugzilla ticket you are working on and add the `feedback?` or `review?` flag depending on if you just want feedback and confirmation you're doing the right thing or if you think the patch is ready to land respectively. Read more about [how to submit a patch and the Bugzilla review cycle here](https://developer.mozilla.org/en-US/docs/Developer_Guide/How_to_Submit_a_Patch).
-
-You can also take a look at the [Code Review Checklist](./code-reviews-checklist.md) as it contains a list of checks that your reviewer is likely to go over when reviewing your code.
-
-### Squashing commits
+# Squashing commits
 
 Sometimes you may be asked to squash your commits. Squashing means merging multiple commits into one in case you created multiple commits while working on a bug. Squashing bugs is easy!
 
 We will use the histedit extension for squashing commits in Mercurial. You can check if this extension is enabled in your Mercurial installation following these steps:
 
 * Open `.hgrc` (Linux/OSX) or `Mercurial.ini` (Windows) –this is the default configuration file of Mercurial– located in your home directory, using your favourite editor.
 * Then add `histedit= ` under the `[extensions]` list present in file, if not present already.