Web Testing/Automation/CodeReviewProcess: Difference between revisions

From MozillaWiki
Jump to navigation Jump to search
Line 67: Line 67:
== Who can do merges? ==
== Who can do merges? ==
* '''For starters, you *cannot* merge-in your own code/pull requests'''
* '''For starters, you *cannot* merge-in your own code/pull requests'''
* Anyone who has tested the patch, made sure it's had two reviews
* Anyone who has tested the patch (and made sure it's had two reviews)

Revision as of 21:58, 30 August 2011

Once code has been pushed into a GitHub branch and a pull request has been created the WebQA team will be notified. So what happens next?

Starting a code review

When doing a code review where should you start?

  • Start by checking that the [StyleGuide]
  • Check that the code meets Python best practises
  • Check that the code looks maintainable and easy to read
  • Make sure you click on the diff button and add comments in line there. It makes life easier for others doing code reviews since they dont need to hunt for your comment.

Who can do a code review?

The whole Mozilla WebQA team can do code reviews and the whole team is encouraged to do code reviews. The reason behind this is that we all learn by new techniques that people think of. It also opens a discussion between the whole team if we feel that something should change.

Who can't do a code review?

You can't review your own code! (Similarly, you also cannot merge in your own code/pull requests.)

How to do a merge?

Once a pull request ready to be pulled the following steps should be followed.

  • Make sure that you have a clone of the Mozilla repository on your local machine
  • git branch pullrequest

This will create another branch (a working copy), called pullrequest, with the code of the current branch)

  • git checkout pullrequest ... for the impatient, git checkout -b pullrequest

Switches to your new branch pullrequest. The reason for doing it in a branch is if there are conflicts it won't break you local master. If you should indeed mess up your branch, you can always create a new branch off your master branch!

  • git remote add requestee requestee_git_fork_url

This command will introduce your clone of the repository with the repository you want to merge with (most of the time, this is the mozilla repository). Example:git remote addmozillahttps://github.com/mozilla/Addon-Tests.gitwill save the Addon-Test repository under the short, and easy-to-remember name mozilla.

  • git fetch requestee && git merge requestee/pull_request_branch_on_their_fork

Get the latest data from the remote repository 'mozilla' and integrate it with your current working branch. Example: git fetch mozilla && git merge mozilla/master will get all the stuff from the mozilla repository (1st step) and then merge the master branch of mozilla with the data of your branch, thus keeping it up-to-date!

  • Run the tests
    • Sometimes good looking code doesn't actually work so lets prove it works!
    • If, and only if, it passes do the next steps
  • git checkout master

Switch to your masterbranch

  • git merge pullrequest master

Merge your branch (working copy)pullrequest with the current master branch

  • git push origin master

Push the changed stuff in your master branch back to your origin repository at github

  • git branch -d pullrequest

Delete the working branch you created, as everything is now merged with your master branch

This will have it all stored in the remote master and you will have a clean version locally. Please **don't** use the merge button on GitHub. You **need** to verify that the merge works and that button can't do that for you!

  • Update the private files if there are changes in them

If the pull request contains changes in the private files update or ask someone to update the private files

  • Ex: Update the webqa-credentials.yml if there are changes in this file

Who can do merges?

  • For starters, you *cannot* merge-in your own code/pull requests
  • Anyone who has tested the patch (and made sure it's had two reviews)