This captures committer-specific information beyond what's covered in the contributions guide.
Accepting Pull Requests on GitHub
The ASF INFRA team maintains a mirror of our git repository over on GitHub. That mirror is strictly one-way: changes from the Apache git get copied over, but not the other way around. Further, the mirror on GitHub is read-only for everyone. Which means we can neither accept nor close pull requests filled there via the GitHub web UI. However, we want to allow for contributions via GitHub Pull Requests. Here's how:
Add the ASF git repository
Code Block | ||
---|---|---|
| ||
$ git remote add apache https://git-wip-us.apache.org/repos/asf/reef.git |
As a result, you can refer to the ASF git as "apache". An example setup, with a the GitHub mirror at upstream and a forked GitHub origin at {username}:
Code Block | ||
---|---|---|
| ||
$ git remote -v
apache https://git-wip-us.apache.org/repos/asf/reef.git (fetch)
apache https://git-wip-us.apache.org/repos/asf/reef.git (push)
origin https://github.com/{username}/reef.git (fetch)
origin https://github.com/{username}/reef.git (push)
upstream https://github.com/apache/reef.git (fetch)
upstream https://github.com/apache/reef.git (push) |
The changes on GitHub gets synced with the changes on GitBox, and vice versa for the changes on GitBox.
Normally, users can file their PRs on the GitHub repository using the GitHub web UI, and merging the PRs through the GitHub web UI will automatically reflect the changes on both the GitHub and the GitBox repositories.
Add the ASF GitBox git repository (optional)
See this guide.
Merge changes
The next step is to merge all changes on the Pull Request as a single commit. There are two methods here: (A) pull the branch from the GitHub Pull Request and squash commits, or (B) get the .diff
from GitHub and manually create a commit from this information. Each option has its pros and cons. With method A, git does some of the tedious work of preserving commit messages; but in some cases the squash may not apply cleanly, and the merger will have to carefully resolve conflicts. With method B, the .diff
will apply cleanly (given that the branch is up-to-date, i.e. the GitHub GUI states that the "This pull request can be automatically merged by project collaborators"); but the merger will have to carefully copy over commit messages and edit author information.
Commit Message
Whichever method you choose, the following should be included in the final commit message:
- Pull request description and commit comments
- A link to the JIRA this is addressing.
- The text "closes #PRNUMBER", where PRNUMBER is the number of the pull request, e.g. "10"
Following the last statement will close the GitHub pull request. It is important to close via the commit message, because only the author can close pull request via the GitHub Web UI.
Example Commit message (80 columns)
Code Block | ||||
---|---|---|---|---|
| ||||
[REEF-33] Allow Tasks to initiate an Evaluator Heartbeat
This adds the class HeartBeatTriggerManager which can be used by a Task to
initiate an Evaluator Heartbeat. It is used by injecting it into the Task.
JIRA:
[REEF-33] https://issues.apache.org/jira/browse/REEF-33
Pull Request:
Closes #24
|
Sometimes, we get pull requests from outside contributors and the Git metadata doesn't contain the proper authorship information. In that case, please add the author information to the commit message, like so:
Code Block | ||||
---|---|---|---|---|
| ||||
[REEF-33] Allow Tasks to initiate an Evaluator Heartbeat
This adds the class HeartBeatTriggerManager which can be used by a Task to
initiate an Evaluator Heartbeat. It is used by injecting it into the Task.
JIRA:
[REEF-33] https://issues.apache.org/jira/browse/REEF-33
Pull Request:
Closes #24
Author:
AuthorName AuthorEmail |
Resolving issue vs Closing issue
The last step of merging a pull request is resolving the associated issue. Committer should resolve it, provide the current release number (the latest unreleased version) in "Fix version" field and provide a link to the pull request in the comment.
Issue should be closed only if no productive steps were taken for it, i.e. it is "won't fix" or "duplicate" or "contained by". If some work was done but it doesn't have pull request associated with it, the issue should be resolved by the person who did the work.
Method A
PowerShell setup instructions contain functions which wrap the following actions and make committer's life easier.
A-1. Create a branch on your local git repository
You want to make sure that that branch is current with the the master branch on Apache git:
Code Block | ||
---|---|---|
| ||
$ git checkout -b BRANCH_NAME
$ git pull apache master |
This assumes that you called the remote git repository at the ASF "apache". You can name the branch however you want, but it is customary to name them either after the pull request number or the JIRA id, e.g. REEF-24.
A-2. Pull the contribution into that branch
This is as simple as
Code Block | ||
---|---|---|
| ||
$ git pull GIT_REPOSITORY_OF_THE_CONTRIBUTOR BRANCH_NAME_OF_THE_CONTRIBUTION |
However, the GitHub web ui makes it somewhat hard to get these two strings. The email from GitHub that informs us of Pull Requests makes this really obvious, though. Consider this quote from pull request #10:
You can merge this Pull Request by running
git pull https://github.com/jwang98052/reef juwang-logfactory
This copies the changes from the given remote branch into the one we just created locally.
Through the GitHub web UI, users can select the option 'Squash and merge` (see screenshots below) upon merging. Please make sure that the pile of commits are squashed with the option, and that commit messages provide meaningful information provided by the author of each of the PRs. In our example below, `[PR-1] Add appropriate whitespace to README (#1)` will show up as the commit message.
Check the commit message
See this guide for what to look for.
...
Check the pull request
- Make sure the code compiles and all tests pass.
- If the pull request changes any POM.xml file: Please either clean your local maven repository before testing OR wait for the build servers to confirm that the project builds. This is to avoid situations in which an artifact was renamed or removed but is still in your local maven repository.
Test java code
At REEF root folder
mvn clean installtitle Run Java Tests Test .Net code
To run tests in command line, go to folder $REEFSourcePath\lang\cs\ and build the projects with the command line:
Code Block title Build source code in debug modemsbuild
Where $REEFSourcePath is REEF source code root folder.
Then run xUnit and mstest tests for each *.Tests.dll. One possible script to do this is:
title Run .Net Tests from command line (Powershell script)$REEFSourcePath = "C:\reef" function Test-REEF-NET-mstest{ Push-Location "$REEFSourcePath\lang\cs\bin\x64\debug" Invoke-Expression 'vstest.console.exe .\Org.Apache.REEF.Client.Tests\Org.Apache.REEF.Client.Tests.dll /Platform:x64' Invoke-Expression 'vstest.console.exe .\Org.Apache.REEF.Tang.Tests\Org.Apache.REEF.Tang.Tests.dll /Platform:x64' Invoke-Expression 'vstest.console.exe .\Org.Apache.REEF.Tests\Org.Apache.REEF.Tests.dll /Platform:x64' Pop-Location } function Test-REEF-NET-xunit{ Push-Location "$REEFSourcePath\lang\cs" $exe = ".\packages\xunit.runner.console.2.1.0\tools\xunit.console.exe" Invoke-Expression "$($exe) $($REEFSourcePath)\lang\cs\bin\x64\Debug\Org.Apache.REEF.Tang.Tests\Org.Apache.REEF.Tang.Tests.dll" Invoke-Expression "$($exe) $($REEFSourcePath)\lang\cs\bin\x64\Debug\Org.Apache.REEF.Client.Tests\Org.Apache.REEF.Client.Tests.dll" Invoke-Expression "$($exe) $($REEFSourcePath)\lang\cs\bin\x64\Debug\Org.Apache.REEF.Wake.Tests\Org.Apache.REEF.Wake.Tests.dll" Invoke-Expression "$($exe) $($REEFSourcePath)\lang\cs\bin\x64\Debug\Org.Apache.REEF.Tests\Org.Apache.REEF.Tests.dll" Invoke-Expression "$($exe) $($REEFSourcePath)\lang\cs\bin\x64\Debug\Org.Apache.REEF.Common.Tests\Org.Apache.REEF.Common.Tests.dll" Invoke-Expression "$($exe) $($REEFSourcePath)\lang\cs\bin\x64\Debug\Org.Apache.REEF.Network.Tests\Org.Apache.REEF.Network.Tests.dll" Invoke-Expression "$($exe) $($REEFSourcePath)\lang\cs\bin\x64\Debug\Org.Apache.REEF.IMRU.Tests\Org.Apache.REEF.IMRU.Tests.dll" Invoke-Expression "$($exe) $($REEFSourcePath)\lang\cs\bin\x64\Debug\Org.Apache.REEF.Evaluator.Tests\Org.Apache.REEF.Evaluator.Tests.dll" Invoke-Expression "$($exe) $($REEFSourcePath)\lang\cs\bin\x64\Debug\Org.Apache.REEF.IO.Tests\Org.Apache.REEF.IO.Tests.dll" Pop-Location }
Alternatively, open $REEFSourcePath\lang\cs\Org.Apache.REEF.sln in VS, rebuild entire solution, then run all tests in Test explore. Note: Visual Studio discovers both xUnit and mstest tests.
If the code touches code that you suspect might break on YARN or Mesos, please test on those environments. If you don't have access to a test environment, ask on the mailing list for help.
Make sure the code adheres to our our coding guidelines.
Make sure that the additions don't create errors in a a Apache RAT check via mvn check via
mvn apache-rat:check
A-4. Rebase the branch onto current apache master
Code Block | ||
---|---|---|
| ||
# Make sure we have the latest bits
$ git fetch apache
# Rebase
$ git rebase -i apache/master |
In the rebase process, make sure that the contribution is squashed to a single commit. From the list of commits, "pick" the commit in the first line (the oldest), and "squash" the commits in the remaining lines:
Code Block | ||
---|---|---|
| ||
pick 7387a49 Comment for first commit
squash 3371411 Comment for second commit
squash 9bf956d Comment for third commit |
Chapter 3 and Chapter 7 of the Git Book contains lots of information on what this means.
Please make sure that the commit message contains the information given in "Commit Message" above. The latest commit can be changed at any time with the command:
Code Block | ||
---|---|---|
| ||
$ git commit --amend |
A-5. Push the code into apache's git
This is a good time to reflect back on this change and whether it is likely to break the build. If you are certain that it won't, go ahead and do:
Code Block | ||
---|---|---|
| ||
$ git checkout master
$ git merge BRANCH_NAME
$ git push apache master |
This pushes the current branch into the master branch hosted on Apache's git repository. From there, it will be mirrored onto GitHub. And by virtue of the comment added above to the commit message, GitHub will now close the Pull Request.
Method B
B-1. Update local master
In this method, you will work directly on your local master branch. Make sure you have the latest changes on your local master branch, by running:
Code Block | ||
---|---|---|
| ||
$ git pull apache master |
B-2. Download the .diff
and apply it
You can download the .diff
file by appending .diff
to the GitHub Pull Request url. This file will contain the exact changes that are shown in "Files changed" in the GitHub GUI. For example, for https://github.com/apache/reef/pull/24
the .diff
file is https://github.com/apache/reef/pull/24.diff
. Using this url, run apply:
Code Block | ||
---|---|---|
| ||
$ wget https://github.com/apache/reef/pull/24.diff
$ git apply 24.diff |
B-3. Commit and edit author information
Commit all files, making sure to include all modified and new files. In the commit message, make sure to include all information given in "Commit Message" above. After committing you must also change the author information to reflect the original author (and not yourself):
Code Block | ||
---|---|---|
| ||
$ git commit --amend --author "Original Author Name <email@address.com>" |
B-4. Push the code into apache's git
Now push your single commit to apache git:
Code Block | ||
---|---|---|
| ||
$ git push apache master |
Resolving issue vs Closing issue
The last step of merging a pull request is resolving the associated issue. Committer should resolve it, provide the current release number (the latest unreleased version) in "Fix version" field and provide a link to the pull request in the comment.
Issue should be closed only if no productive steps were taken for it, i.e. it is "won't fix" or "duplicate" or "contained by". If some work was done but it doesn't have pull request associated with it, the issue should be resolved by the person who did the workThis pushes the current branch into the master branch hosted on Apache's git repository. From there, it will be mirrored onto GitHub. And by virtue of the comment added above to the commit message, GitHub will now close the Pull Request.