Differences between revisions 9 and 10
⇤ ← Revision 9 as of 2017-03-02 15:52:24 →
Size: 3934
Editor: JendrikSeipp
Comment:
← Revision 10 as of 2020-07-08 17:59:52 → ⇥
Size: 3698
Editor: FlorianPommerening
Comment: update rietveld instructions
Deletions are marked like this. Additions are marked like this.
Line 3: Line 3:
= Code reviews with Bitbucket = = Code reviews with Github =
Line 7: Line 7:
 * Prepare your issue or feature branch (e.g. issue123) according to [[../Mercurial|our Mercurial workflow]].
 * Push your Fast Downward repository to Bitbucket and navigate to it in your browser.
 * Click "Pull request" and select the issue123 branch on the left and the "default" branch (preselected) on the right.
 * Preview the changes by clicking on the "Diff" header.
 * Add reviewers and click "Create pull request".		  * Prepare your issue or feature branch (e.g. issue123) according to [[../Git|our Git workflow]].
 * Push your Fast Downward repository to Github and navigate to it in your browser.
 * Click "Pull requests" and click "New pull request", then select the `main` branch of '''your repository''' on the left and the `issue123` branch on the right.
 * Click "Create pull request".
Line 16: Line 15:
Compare any two revisions by pasting them into the following template URL in '''reverse''' order:

{{{https://bitbucket.org/<OWNER>/<PROJECT>/branches/compare/<commit2>..<commit1>#diff}}}, e.g.,

https://bitbucket.org/jendrikseipp/downward/branches/compare/issue693-v2..issue693-v1#diff		 Compare any two revisions by following the steps above but using the commit IDs of the commits you want to compare instead of the branch names. It is not possible to create a pull request this way because merging might not make sense for the selected commits but you'll be able to see the code difference.
Line 29: Line 24:
 * Writing a Mercurial extension (or plain shell script) that automatically calls Rietveld's {{{upload.py}}} with the appropriate info for [[../Mercurial|our Mercurial workflow]].  * Writing a Git hook (or plain shell script) that automatically calls Rietveld's {{{upload.py}}} with the appropriate info for [[../Git|our Git workflow]].
Line 31: Line 26:
 * Writing a Mercurial extension that automatically uploads a patch to Roundup attached to the appropriate issue (from where a Roundup/Rietveld integration plugin could take over). Could also go the other way: automatically fetch a patch from an issue and apply it.
 * Automatically matching up our Roundup users to Rietveld users, or automatically matching up our Mercurial users to Rietveld users (e.g. linking the nosy list of an issue to the "reviewers" of a Rietveld issue.)		  * Writing a Git hook that automatically uploads a patch to Roundup attached to the appropriate issue (from where a Roundup/Rietveld integration plugin could take over). Could also go the other way: automatically fetch a patch from an issue and apply it.
 * Automatically matching up our Roundup users to Rietveld users, or automatically matching up our Git users to Rietveld users (e.g. linking the nosy list of an issue to the "reviewers" of a Rietveld issue.)
Line 44: Line 39:
 1. {{{hg update}}} to the "new" version of the code that is to be reviewed. If you're working on issue1000, {{{
hg update issue1000}}} should do the trick.
 1. Find out (using {{{hg log}}}, {{{hg parents}}} etc.) which "base" version of the code this should be reviewed against. Normally this would be the parent of the first revision on your branch, which you can find out with {{{
hg parents -r $(hg log --branch issue1000 --template '{node}\n' | tail -1)}}} (Is there an easier way?) Let {{{BASE}}} be the changeset identified in this way. You can use either the local numeric id or the global hash.		 /!\ The following instructions are untested for Git.

 1. {{{git checkout}}} to the "new" version of the code that is to be reviewed. If you're working on issue1000, {{{
git checkout issue1000}}} should do the trick.
 1. Find out (using {{{git log}}}, etc.) which "base" version of the code this should be reviewed against. Normally this would be the parent of the first revision on your branch. Let {{{BASE}}} be the changeset identified in this way. You can use either the local numeric id or the global hash.
Line 49: Line 45:
hg meld -r BASE}}} and verify that you picked the correct {{{BASE}}} revision. (If you haven't set up {{{hg meld}}} yet, see [[../Mercurial]].) git diff BASE}}} and verify that you picked the correct {{{BASE}}} revision.

Back to developer page.

Code reviews with Github

You can prepare a code review by making a pull-request in your own Fast Downward repository on Bitbucket.

  • Prepare your issue or feature branch (e.g. issue123) according to our Git workflow.

  • Push your Fast Downward repository to Github and navigate to it in your browser.

  • Click "Pull requests" and click "New pull request", then select the main branch of your repository on the left and the issue123 branch on the right.

  • Click "Create pull request".
  • Link to the pull request from the issue tracker.

Compare arbitrary revisions

Compare any two revisions by following the steps above but using the commit IDs of the commits you want to compare instead of the branch names. It is not possible to create a pull request this way because merging might not make sense for the selected commits but you'll be able to see the code difference.

Code reviews with Rietveld

We have also been using the Rietveld code review tool.

There are various fancy ways in which Rietveld could be integrated into our infrastructure (repository, issue tracker), such as:

  • Hosting our own instance of Rietveld rather than the one hosted by Google.

  • Writing a Git hook (or plain shell script) that automatically calls Rietveld's upload.py with the appropriate info for our Git workflow.

  • Writing a Roundup plugin that identifies patches that are attached to an issue and automatically creates and links a matching Rietveld issue.
  • Writing a Git hook that automatically uploads a patch to Roundup attached to the appropriate issue (from where a Roundup/Rietveld integration plugin could take over). Could also go the other way: automatically fetch a patch from an issue and apply it.
  • Automatically matching up our Roundup users to Rietveld users, or automatically matching up our Git users to Rietveld users (e.g. linking the nosy list of an issue to the "reviewers" of a Rietveld issue.)

For now, let's leave these as potential avenues to explore in the future and stick with using Rietveld manually until we've got the hang of it.

Using Rietveld manually

To get set up, download Rietveld's upload.py with 

wget http://codereview.appspot.com/static/upload.py

and store it somewhere on your path. Don't forget to make it executable.

To create a patch:

/!\ The following instructions are untested for Git.

  1. git checkout to the "new" version of the code that is to be reviewed. If you're working on issue1000, 

    git checkout issue1000
    should do the trick.

  2. Find out (using git log, etc.) which "base" version of the code this should be reviewed against. Normally this would be the parent of the first revision on your branch. Let BASE be the changeset identified in this way. You can use either the local numeric id or the global hash.

  3. Optionally, run 

    git diff BASE

    and verify that you picked the correct BASE revision.

  4. Run 

    upload.py --rev=BASE
    to create a Rietveld issue.

  5. You'll have to answer some questions. When queried for your email and password, use ones that are tied to some Google account (e.g. that you use for Google groups); they don't have to be gmail/googlemail addresses.

  6. You'll see a line like 

    Issue created. URL: http://codereview.appspot.com/XXXXXXX
    that provides the URL for the newly created issue. Send it to the potential reviewer or go to that page to invite a reviewer via Rietveld's web interface.

FastDownward: ForDevelopers/CodeReview (last edited 2020-07-09 08:47:09 by SilvanSievers)