Skip to content

Commit

Permalink
Update walkthrough to use moz-phab and mozilla-central
Browse files Browse the repository at this point in the history 
Summary:
Update the walkthrough to use moz-phab.  Also update examples and
screenshots to use real Firefox source tree information.

Test Plan:
To view the final HTML documentation follow the instructions in
the conduit-docs project README.

Reviewers: glob, smacleod, zalun

Reviewed By: glob, zalun

Bug #: 1525284

Differential Revision: https://phabricator.services.mozilla.com/D21093
  • Loading branch information
@mars-f
mars-f committed Feb 28, 2019
1 parent bf54dad commit 60de383
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 233 deletions.
Binary file modified images/lando-land-it.png
View file
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified images/view-in-lando.png
View file
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
309 changes: 76 additions & 233 deletions walkthrough.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,187 +2,29 @@
Phabricator Workflow Walk-through
*********************************

**IMPORTANT:** Make sure you have :ref:`set up Phabricator and Arcanist <quick-start>` before proceeding!
**IMPORTANT:** Make sure you have :ref:`set up Phabricator <creating-account>` and
:ref:`set up Arcanist <setting-up-arcanist>` before proceeding!

Multi-Head vs. bookmarks
========================
While some developers use bookmarks/etc to track changes, for this this guide we will
use just repository heads: no bookmarks or labels. This essentially means "just
start coding off tip and commit". The ``hg wip`` alias provides a view of the
repository that allows for keeping track of the work.

While some developers use bookmarks/etc to track changes, it's possible to just create a new "head", which essentially means "just start coding off tip and commit". The ``hg wip`` alias provides a view of the repository that allows for keeping track of the work.
(If you want to dig deeper into the "to label or not to label" discussion,
see `this document <https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/workflows.html#to-label-or-not-to-label>`_)

For this guide we will use just repository heads: no bookmarks or labels.
Landing a commit to mozilla-central
===================================

(If you want to dig deeper into the "to label or not to label" discussion, see `this document <https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/workflows.html#to-label-or-not-to-label>`_)


Style 1: One repository head per review
=======================================

In this style we use GitHub-style fix-up commits under a single repository head. The fix-ups will be squashed into a single commit before landing.

We'll use:

* One repository head
* One review request per head
* ``hg commit`` to add fix-up commits to the head

Fixing the code
---------------

Let's start with a clean checkout.

::

$ hg wip
@ 5865 tip @ autoland: configure lando s3 bucket (bug 1448051).
|
~

$ vim pylib/mozautomation/mozautomation/commitparser.py
$ hg status
M pylib/mozautomation/mozautomation/commitparser.py

Make sure our commit message is well-formatted.

* We do not need a bug number or "r?" reviewers list. Lando will add this automatically.

::

$ hg commit
Fix pep8 lint

Fix some PEP 8 lint in the module level docstrings.

Oops, we found a second file to update. We can add a fix-up commit to the head that adds the new changes.

::

$ vim pylib/mozautomation/mozautomation/treestatus.py
$ hg commit -m 'docstring for the treestatus module'
$ hg wip
@ 5871 tip docstring for the treestatus module
o 5870 Fix pep8 lint
o 5865 @ autoland: configure lando s3 bucket (bug 1448051).
o 5864 hgmo: upgrade ZooKeeper to 3.4.11 (bug 1434339) r=sheehan
o 5863 autoland: configure lando pingback url (bug 1445567) (fixup)
|


Requesting a Review
-------------------

Before we request a review we should check for changes upstream.

::

$ hg pull --rebase

* If you want to look before you leap in with ``arc diff`` or ``arc land``, you can run ``arc which`` to get a description of what ``arc`` is going to do next.

We need to include:

* A real BMO Bug #
* Reviewers

The Summary, Test Plan, and Subscribers sections are optional.

::

$ arc diff
Fix pep8 lint

Summary:

Fix some PEP 8 lint in the module level docstrings.

Test Plan: $ pep8 thefiles

Reviewers: glob, imadueme

Subscribers:

Bug #: 5556555

# NEW DIFFERENTIAL REVISION
# Describe the changes in this new revision.
#
# Included commits in branch default:
#
# 153ddf055585 docstring for the treestatus module
# c7ab40d66585 Fix pep8 lint
#
# arc could not identify any existing revision in your working copy.
# If you intended to update an existing revision, use:
#
# $ arc diff --update <revision>


Addressing feedback
-------------------

Our reviewers came back with some changes. Let's add some fix-up commits for the work.

::

$ hg wip
o 5871 tip docstring for the treestatus module
o 5870 Fix pep8 lint
@ 5865 @ autoland: configure lando s3 bucket (bug 1448051).

$ hg checkout 5871
$ vim pylib/mozautomation/mozautomation/treestatus.py
# hack hack
$ hg commit -m 'fix lint'


Check off the Done item in the Phabricator UI.

.. image:: images/review-item-done.png
:align: center
:alt: Screenshot of a Done review item

Now run ``arc diff``. Phabrictor will automatically submit your Done items in the UI and create a nicely formatted update.

::

$ arc diff

.. image:: images/done-items-update.png
:align: center
:alt: Screenshot of a revision history update after pushing updates


Landing the changes
-------------------

Everything looks good: the reviewers have approved our changes. Let's land our changes in mainline.

On your revision page in Phabricator click the "View in Lando" link in the right-hand menu:

.. image:: images/view-in-lando.png
:align: center
:alt: Screenshot of a Phabricator Revision ready to land with Lando


You will be taken to the Lando revision overview page. Give the change one last review, double-check the commit message, etc., before hitting the "Land" button.

.. image:: images/lando-land-it.png
:align: center
:alt: Screenshot of a revision in Lando that is ready to land

Hit the "Land" button and Lando will automatically commit your changes to mainline.



Style 2: One changeset per review
=================================

In this style we craft just one commit per review. When we get feedback or fixups we amend our single commit.
For this example we will craft one change for review. When we get feedback that changes
are required we will amend our commit in-place.

We'll use:

* One commit
* One review request per commit
* ``hg amend`` to add fix-ups to our commit
* ``moz-phab`` to request code reviews


Fixing the code
Expand All @@ -193,22 +35,34 @@ Let's start with a clean checkout.
::

$ hg wip
@ 5865 tip @ autoland: configure lando s3 bucket (bug 1448051).
|
~
@ 460880 tip Merge inbound to mozilla-central. a=merge
|\
~ ~

$ vim pylib/mozautomation/mozautomation/commitparser.py
$ vim dom/audiochannel/AudioChannelAgent.cpp
# hack hack

$ hg status
M pylib/mozautomation/mozautomation/commitparser.py
M dom/audiochannel/AudioChannelAgent.cpp

Make sure our commit message is well-formatted.

* We do not need a bug number or "r?" reviewers list. Lando will add this automatically.
When we write the commit message we should follow the Firefox source tree's common
commit message format. We will include a Bug ID and list of reviewers. See the
`committing rules <https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities#Commit_message_restrictions>`_
for more information about proper commit message formatting.

::

$ hg commit
$ hg commit
Bug 1445923 - WebAudio: Remove b2g dead code r?sylvestre


Removed dead code from ...

* You are allowed to skip the bug number and reviewer names if you don't have them yet,
but the `Lando automated landing system <https://lando.services.mozilla.com/>`_,
used later in this walk-through, will insist that you add them before the code can
be landed.


Requesting a Review
-------------------
Expand All @@ -219,104 +73,93 @@ Before we request a review we should check for changes upstream.

$ hg pull --rebase

* If you want to look before you leap in with ``arc diff`` or ``arc land``, you can run ``arc which`` to get a description of what ``arc`` is going to do next.

We need to include:

* A real BMO Bug #
* Reviewers

The Summary, Test Plan, and Subscribers sections are optional.
The ``moz-phab`` command will take care of creating a code review for us. It will
automatically link the review to the BMO bug as well as notifying the reviewers.

::

$ arc diff
Fix pep8 lint

Summary:

Fix some PEP 8 lint in the module level docstrings.

Test Plan: $ pep8 thefiles

Reviewers: glob, imadueme

Subscribers:
$ moz-phab
Submitting 1 commit:
(New) 460880:e376ef5bf453 Bug 1445923 - WebAudio: Remove b2g dead code r?sylvestre
Submit to Phabricator (YES/No/Always)?

Bug #: 5556555
...

# NEW DIFFERENTIAL REVISION
# Describe the changes in this new revision.
#
# Included commits in branch default:
#
# 153ddf055585 docstring for the treestatus module
# c7ab40d66585 Fix pep8 lint
#
# arc could not identify any existing revision in your working copy.
# If you intended to update an existing revision, use:
#
# $ arc diff --update <revision>
Completed
(D55555) 460880:e376ef5bf453 Bug 1445923 - WebAudio: Remove b2g dead code r?sylvestre
-> https://phabricator.services.mozilla.com/D55555


Addressing feedback
-------------------

When it's time to address feedback we use ``hg amend``.

* ``hg commit --amend`` also works, and allows you to update the commit description while amending the commit
* ``hg commit --amend`` also works, and allows you to update the commit description
while amending the commit.

::

$ hg wip
o 5870 tip Fix pep8 lint
@ 5865 @ autoland: configure lando s3 bucket (bug 1448051).
o 5864 hgmo: upgrade ZooKeeper to 3.4.11 (bug 1434339) r=sheehan
o 5863 autoland: configure lando pingback url (bug 1445567) (fixup)
|

$ hg checkout 5870
$ vim pylib/mozautomation/mozautomation/commitparser.py
# hack hack
@ 460881 tip Bug 1445923 - WebAudio: Remove b2g dead code r?sylvestre
o 460880 Merge inbound to mozilla-central. a=merge
|\
~ ~


$ hg checkout 460881
$ vim dom/audiochannel/AudioChannelAgent.cpp
# fixup fixup

$ hg amend


Check off the Done item in the Phabricator UI.

.. image:: images/review-item-done.png
:width: 800
:align: center
:alt: Screenshot of a Done review item


Now run ``arc diff``. Phabrictor will automatically submit your Done items in the UI and create a nicely formatted update.
Now run ``moz-phab`` a second time. Phabricator will automatically submit your Done
items in the UI and notify your reviewers that you have made updates.

::

$ arc diff
::

.. image:: images/done-items-update.png
:align: center
:alt: Screenshot of a revision history update after pushing updates
$ moz-phab


Landing the changes
-------------------

Everything looks good: the reviewers have approved our changes. Let's land our changes in mainline.
Everything looks good: the reviewers have approved our changes. Let's land our
changes.

On your revision page in Phabricator click the "View in Lando" link in the right-hand menu:
On your revision page in Phabricator click the "View Stack in Lando" link in the
right-hand menu:

.. image:: images/view-in-lando.png
:width: 800
:align: center
:alt: Screenshot of a Phabricator Revision ready to land with Lando


You will be taken to the Lando revision overview page. Give the change one last review, double-check the commit message, etc., before hitting the "Land" button.
You will be taken to the Lando revision overview page. Press "Preview Landing" and give
the change one last review: double-check the commit message, etc., before hitting the
"Land" button.

.. image:: images/lando-land-it.png
:width: 800
:align: center
:alt: Screenshot of a revision in Lando that is ready to land

Hit the "Land" button and Lando will automatically commit your changes to mainline.
Hit the "Land" button and Lando will automatically land your changes.

Where to go from here
=====================

``moz-phab`` has other features including the ability to update and land entire stacks
of related commits. Check out the :ref:`Submitting Patches <using-moz-phab>` section
for more information.

0 comments on commit 60de383

Please sign in to comment.