Web Testing/Automation/CodeReviewProcess: Difference between revisions

Line 31: Line 31:
Once the pull request has received the required number of positive reviews (usually one or two), squash your commits locally and push a new single commit to your fork to replace all of the commits that were created during the review process.
Once the pull request has received the required number of positive reviews (usually one or two), squash your commits locally and push a new single commit to your fork to replace all of the commits that were created during the review process.


== How to do a merge? ==
== Merging a pull request ==


Once a pull request ready to be pulled the following steps should be followed.
Before merging a pull request do the following:


*Make sure that you have a clone of the Mozilla repository on your local machine
* Make sure all the tests are still passing.
*'''git branch ''pullrequest'''''
* Make sure the pull request has at least one positive review.
* Make sure that the commits in the pull request have been appropriately squashed.
** If they have not been, ask the submitter to do so.
** Note that if the review process was particularly lengthy and resulted in multiple changes it may make sense to keep more than one commit, but in general commits should be squashed.


This will create another branch (a working copy), called ''pullrequest'', with the code of the current branch)
After verifying the above, click the big green "Merge pull request" button in Github.


*'''git checkout '''''<b>pullrequest</b>'' ... for the impatient, '''git''' '''checkout -b ''pullrequest'''''<br>
After the merge is complete please watch the next build in Jenkins to ensure that it is green, or at least adds no new failures.
 
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!<br>
 
*'''git remote add '''''<b>requestee requestee_git_fork_url</b>''
 
''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:'''''<b>git </b>'''remote add'''''<b>mozilla</b>'''https://github.com/mozilla/Addon-Tests.git'''''will save the Addon-Test repository under the short, and easy-to-remember name ''mozilla''.<br>''
 
*'''git fetch''' '''''requestee''''' '''&amp;&amp; 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'' &amp;&amp; git merge '''''<b>mozilla/master</b>'' 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<br>
 
*'''git checkout '''''<b>master</b>''
 
''Switch to your ''master''branch''
 
*'''git merge '''''<b>pullrequest master</b>''
 
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 '''''<b>pullrequest</b>''
 
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!
 
*<b>Update the private files if there are changes in them</b>
 
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
 
'''IMPORTANT''': Post-merge, stick around to ensure that subsequent build is green, or at least adds no new failures


== 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 (and made sure it's had two reviews)
* Anyone who has tested the patch (and made sure it's had two reviews)
Confirmed users
425

edits