Difference between revisions of "Submitting Contributions"

From WebOS-Ports
Jump to navigation Jump to search
(Created page with "== Overview == As webOS Ports is a community project we're very happy about accepting patches. But there are some things you have to know before and to respect when submitting p...")
 
 
(13 intermediate revisions by 5 users not shown)
Line 2: Line 2:
  
 
As webOS Ports is a community project we're very happy about accepting patches. But there are some things you have to know before and to respect when submitting patches.
 
As webOS Ports is a community project we're very happy about accepting patches. But there are some things you have to know before and to respect when submitting patches.
 +
 +
The process is only for components hosted on the webOS ports github project (http://github.com/webOS-ports) and are specific to webOS ports. For contributions which belong to the Open webOS project please read the guideline at [http://www.openwebosproject.org/community/accept/#.UhiVCd8ofn4 How We Accept Contributions].
  
 
==How We Accept Contributions==
 
==How We Accept Contributions==
 +
 +
When submitting pull requests to webOS Ports projects, developers are asked to follow these guidelines. This will make sure we can bring your code in with little hassle.
 +
 +
==Generic Instructions/Guidelines==
 +
 +
Please follow the guidelines below to ensure that your contributions can quickly and easily be reviewed and pulled in to the official WOCE codebase. These are mostly standard Github practices, codified and documented on this wiki so that contributors have a single source of WOCE documentation.
 +
 +
=== Pull Requests should merge with their branch cleanly ===
 +
 +
Project maintainers should not have to make your code merge cleanly. Before pull requesting, please merge against the branch they are on:
 +
 +
git pull [email protected]:woce/LunaSysMgr
 +
 +
... and [http://genomewiki.ucsc.edu/index.php/Resolving_merge_conflicts_in_Git clean up any merge conflicts] yourself. If there are any questions you have when merging ask the repository maintainers on a best course of action on resolving the conflict. Do not just take your code and discard the code in master unless you understand what that code is doing and are sure nothing will be broken.
 +
 +
After cleaning up the merge, compile and test your changes to make sure you didn't break anything.
 +
 +
'''Project maintainers should not have to make your code merge cleanly'''
 +
 +
If multiple people are working within the same classes and it is difficult to keep merges automatically working, one of the developers should wait until the other has completed their pull request to file a new one. This keeps the chances of clobbering each others' work to a minimum
 +
 +
=== Commits ===
 +
Commit messages should be descriptive, and follow this convention:
 +
  [component name]: [single-sentence overview]
 +
 
 +
  * [point one]
 +
  * [point two]
 +
  * [point three]
 +
  * [and so on]
 +
 
 +
  Open-WebOS-DCO-1.0-Signed-Off-By: [your name] <[your email]>
 +
 +
Additionally, the number of commits should be kept to a sensible level. Rather than making many commits with messages like 'Changed variable foo' or 'Removed function bar', developers should make one commit per 'task'.
 +
 +
Example Commit:
 +
 +
  Input Window Manager: Use DPI Scaling
 +
 
 +
  * Create keyboard using DPI-scaled resolution
 +
  * Use LunaSettings->dpi instead of QApplication::desktop dpi
 +
    (For consistency, allows for tweaking if device dpi != desired dpi)
 +
  * Also fix some code indentation issues
 +
 
 +
  Open-WebOS-DCO-1.0-Signed-Off-By: Josh Palmer <[email protected]>
 +
 +
=== Each pull request should be for a single feature or bug fix ===
 +
 +
Do not include multiple features or fixes in each bug fix. This makes your code easier to review and will get your requests merged quicker.
 +
 +
If your feature relies on changes to or additions of a library or header file, that should be done in a seperate pull request prior to the feature itself. This not only prevents other people who want to build on top of your library to work normally but also allows you to work on a new feature should the feature you're working on be kept from merging for some reason or another.
 +
 +
=== Pull request summaries should be descriptive ===
 +
 +
A corollary to the above is that pull requests summaries should be descriptive. "Update LunaSysMgr" pull requests will probably be denied.
 +
 +
=== Commits should be atomic ===
 +
 +
Commit every time you change something. Commit messages should be descriptive but at the same time concise. This means that you should commit every step of the way that you get something that works.
 +
 +
This makes resolving merge conflicts easier and will make your life easier in general. When it comes to bisecting to figure out where bugs come, atomic commits are also important since it leaves you with a smaller code base to manually inspect.
 +
 +
=== Suggested Workflow ===
 +
 +
Based on the above, we recommend working in feature branches for each of your pull requests, all of them based off of your master and have drafted this recommended workflow:
 +
 +
# on branch master
 +
git pull [email protected]:woce/LunaSysMgr # make sure that your master is up to date with upstream (WOCE) master
 +
git checkout -b cards-magic-arrangement # create a branch called cards-magic-arrangement and check it out
 +
# on branch card-mangic-arrangement
 +
# change a few things
 +
git add source/arranger_class_bar.h
 +
git commit -m "Add new methods for the magic arranger to header file"
 +
# change more stuff
 +
git add source/arranger_class_bar.cpp
 +
git commit -m "Make cards magically arrange"
 +
git add source/magic_arranger_helper.cpp
 +
git commit -m "add a helper class for the magic arranger"
 +
# You're done with your feature, now let's get ready to pull request!
 +
git pull [email protected]:woce/LunaSysMgr # this gets us back up to date with upstream master
 +
# if there are any merge conflicts, solve them a la http://genomewiki.ucsc.edu/index.php/Resolving_merge_conflicts_in_Git
 +
git push origin card-magic-arranger
 +
 +
<!--T:2-->
 +
{{tip|tip=You can use <code>git add --patch</code> or <code>git add -p</code> (especially its command <code>s</code> (which stands for ''split'') for commiting partial patches. This allows clean commits without <code>git stash</code>es.}}
 +
 +
You now have a branch on your github fork which you can pull request in to WOCE master or WOCE features. When you pull request to the Github fork, you should see only the following commits (obviously they only apply to this example):
 +
 +
* Add new methods for the magic arranger to header file
 +
* Make cards magically arrange
 +
* add a helper class for the magic arranger
 +
* Merge branch master of [email protected]:woce/LunaSysMgr
 +
 +
The last commit will only be shown if there was something to merge when you make that last git pull before pushing.
 +
 +
==Tab Characters==
 +
All edits to C/++/JS webOS Ports projects should be made using tab characters for indentation, '''not''' spaces.
 +
 +
Most text editors provide a setting for this, and vim users can :set the following options:
 +
  noet
 +
  ts=4
 +
  sw=4
 +
 +
Additionally, you can convert an entire file to using tab characters in vim by running the following:
 +
  :set noet ts=4 |%retab!
 +
 +
==Commits==
 +
 +
 +
 +
==Branch Layout==
 +
Branches should have some degree of organization to them- it's not a good idea to work directly on top of your upstream tracking branch (webOS-ports/master in most cases). Ideally you shouldn't commit anything to it, and use it as a reference to base your other 'feature' branches on. That way you can git pull and rebase your feature branches on it in order to stay up-to-date.
 +
 +
==Rebasing==
 +
Before issuing a pull request, developers should update their upstream/master tracking branch (webOS-ports/master in most cases) and then rebase their feature branch on it with the following commands:
 +
  git checkout webOS-ports/master
 +
  git pull
 +
  git checkout <yourbranch>
 +
  git rebase <yourbranch> webOS-ports/master
 +
  <fix any merge conflicts and commit>
 +
  git push origin <yourbranch>
 +
 +
Example:
 +
  git checkout webOS-ports/master
 +
  git pull
 +
  git checkout shiftyaxel/ui-scaling
 +
  git rebase shiftyaxel/ui-scaling webOS-ports/master
 +
  <fix any merge conflicts and commit>
 +
  git push origin shiftyaxel/ui-scaling
 +
 +
This will make sure that your branch can be merged with ours without any conflicts, because you will have fixed them before issuing the Pull Request.
 +
 +
Once you have run the above commands, your Github repo page will show your feature branch under 'recently pushed' with a button to initiate a Pull Request.
 +
 +
==Reworking==
 +
Sometimes a Pull Request won't quite work with what webOS Ports has in mind, and you'll be asked to rework it in some way. When doing this, you can still use your feature branch- there's no need to close the pull request and reopen a new one.
 +
 +
Any changes pushed to your feature branch will show in the existing pull request, where you can continue the discussion with the team and move towards getting your Pull Request approved.
 +
  
 
===The Contribution Process===
 
===The Contribution Process===
  
 
For big changes, the contributor communicates with the Project via mailing lists or Bug Tracker tickets to get feedback before submitting code
 
For big changes, the contributor communicates with the Project via mailing lists or Bug Tracker tickets to get feedback before submitting code
*Contributor adds the Open webOS DCO signoff to each commit message during development - see the link for format and meaning
+
*Contributor adds signoff line to each commit message during development  
*Contributor makes a GitHub pull request
+
*Contributor makes a GitHub pull request (see https://help.github.com/articles/creating-a-pull-request)
*Maintainer conducts code review, verifies DCO signoff, runs tests and asks for adjustments from contributor as necessary
+
*Maintainer conducts code review, verifies signoff, runs tests and asks for adjustments from contributor as necessary
 
*Maintainer merges the commits into the repo and closes the pull request
 
*Maintainer merges the commits into the repo and closes the pull request
  
Line 43: Line 183:
 
Assuming no one has forked from your fork, or you don't mind breaking them use "rebase -i" to edit the existing commit messages (Google it)
 
Assuming no one has forked from your fork, or you don't mind breaking them use "rebase -i" to edit the existing commit messages (Google it)
 
Open a new pull request with the fixed commits
 
Open a new pull request with the fixed commits
Close the existing pull request, with a comment showing the new one that it has superseded
+
Close the existing pull request, with a comment showing the new one that it has superseeded.
If you'd like to discuss what's required, please join us on the openwebos-general mailing list, starting with searching the archives on this topic (e.g. Google "site:mailman.openwebosproject.org DCO signoff").
+
 
 +
==Including changes in build==
 +
After a pull request has been closed and committed to the branch, it still needs to be updated in the recipe as well, so it's included in the future build.
 +
 
 +
Recipes can be found at https://github.com/webOS-ports/meta-webos-ports/ we're currently using the Dora branch which resides at https://github.com/webOS-ports/meta-webos-ports/tree/dora/
 +
 
 +
For the apps, the recipes can be found at https://github.com/webOS-ports/meta-webos-ports/tree/dora/recipes-webos/apps
 +
 
 +
There are different kinds of recipes
 +
 
 +
===Recipes with SRCREV===
 +
 
 +
These recipes are the most straight forward. Just update the SRCREV tag in the recipe with the SHA value of the respective commit for the app/component.
 +
 
 +
For example:
 +
 
 +
  SRCREV = "c8500b2040e0c8deceeaba369a04a4ba47e82d9c"
 +
 
 +
in https://github.com/webOS-ports/meta-webos-ports/blob/dora/recipes-webos/apps/org.webosinternals.preware.bb refers to commit https://github.com/webOS-ports/preware/commit/c8500b2040e0c8deceeaba369a04a4ba47e82d9c
 +
 
 +
Updating it to commit f69ab1b282eb77a11d4397e03575790871c755bb can be done by simply replacing
 +
 
 +
  SRCREV = "c8500b2040e0c8deceeaba369a04a4ba47e82d9c"
 +
 
 +
with
 +
 
 +
  SRCREV = "f69ab1b282eb77a11d4397e03575790871c755bb"
 +
 
 +
===Recipes with WEBOS_VERSION===
 +
These recipes are a bit more tricky since they include release, submission as well as SHA.
 +
 
 +
For example in https://github.com/webOS-ports/meta-webos-ports/blob/dora/recipes-webos/apps/org.webosports.app.phone.bb
 +
 
 +
  WEBOS_VERSION = "0.1.0-3_6f4262951247971b3857ab95c04470f130cb18e0"
 +
 
 +
0.1.0 refers to release, 3 refers to submission and 6f4262951247971b3857ab95c04470f130cb18e0 refers to the SHA for the submission tag.
 +
 
 +
All these 3 values need updating in case of a new release or submission.
 +
 
 +
A submission always corresponds to a tag in the git repository in the format "submission/<number>". To create a new submission tag for a specific commit or just the HEAD of the git repository use the following command and replace <number> and <commit rev>:
 +
 
 +
  git tag submissions/<number> -m "submissions/<number>" <commit rev>
 +
 
 +
Don't forget to push the tag afterwards
 +
 
 +
  git push origin submissions/<number>
 +
 
 +
After the submission tag is created you can show its rev with the following command
 +
 
 +
  git show-ref submissions/<number>
 +
 
 +
The returned SRCREV is what has to be added to the WEBOS_VERSION variable as last part.
 +
 
 +
'''NOTE:''' If the build complains about the tag being on a commit on the wrong branch go and recreate the tag on the correct branch (don't forget to remove the tag before).
 +
 
 +
'''NOTE:''' After a tag is once submitted to a build the tag should never be touched again. The build will notice any modifications to the created tag and will fail!

Latest revision as of 23:32, 30 September 2014

Overview

As webOS Ports is a community project we're very happy about accepting patches. But there are some things you have to know before and to respect when submitting patches.

The process is only for components hosted on the webOS ports github project (http://github.com/webOS-ports) and are specific to webOS ports. For contributions which belong to the Open webOS project please read the guideline at How We Accept Contributions.

How We Accept Contributions

When submitting pull requests to webOS Ports projects, developers are asked to follow these guidelines. This will make sure we can bring your code in with little hassle.

Generic Instructions/Guidelines

Please follow the guidelines below to ensure that your contributions can quickly and easily be reviewed and pulled in to the official WOCE codebase. These are mostly standard Github practices, codified and documented on this wiki so that contributors have a single source of WOCE documentation.

Pull Requests should merge with their branch cleanly

Project maintainers should not have to make your code merge cleanly. Before pull requesting, please merge against the branch they are on:

git pull [email protected]:woce/LunaSysMgr

... and clean up any merge conflicts yourself. If there are any questions you have when merging ask the repository maintainers on a best course of action on resolving the conflict. Do not just take your code and discard the code in master unless you understand what that code is doing and are sure nothing will be broken.

After cleaning up the merge, compile and test your changes to make sure you didn't break anything.

Project maintainers should not have to make your code merge cleanly

If multiple people are working within the same classes and it is difficult to keep merges automatically working, one of the developers should wait until the other has completed their pull request to file a new one. This keeps the chances of clobbering each others' work to a minimum

Commits

Commit messages should be descriptive, and follow this convention:

 [component name]: [single-sentence overview]
 
 * [point one]
 * [point two]
 * [point three]
 * [and so on]
 
 Open-WebOS-DCO-1.0-Signed-Off-By: [your name] <[your email]>

Additionally, the number of commits should be kept to a sensible level. Rather than making many commits with messages like 'Changed variable foo' or 'Removed function bar', developers should make one commit per 'task'.

Example Commit:

 Input Window Manager: Use DPI Scaling
 
 * Create keyboard using DPI-scaled resolution
 * Use LunaSettings->dpi instead of QApplication::desktop dpi
   (For consistency, allows for tweaking if device dpi != desired dpi)
 * Also fix some code indentation issues
 
 Open-WebOS-DCO-1.0-Signed-Off-By: Josh Palmer <[email protected]>

Each pull request should be for a single feature or bug fix

Do not include multiple features or fixes in each bug fix. This makes your code easier to review and will get your requests merged quicker.

If your feature relies on changes to or additions of a library or header file, that should be done in a seperate pull request prior to the feature itself. This not only prevents other people who want to build on top of your library to work normally but also allows you to work on a new feature should the feature you're working on be kept from merging for some reason or another.

Pull request summaries should be descriptive

A corollary to the above is that pull requests summaries should be descriptive. "Update LunaSysMgr" pull requests will probably be denied.

Commits should be atomic

Commit every time you change something. Commit messages should be descriptive but at the same time concise. This means that you should commit every step of the way that you get something that works.

This makes resolving merge conflicts easier and will make your life easier in general. When it comes to bisecting to figure out where bugs come, atomic commits are also important since it leaves you with a smaller code base to manually inspect.

Suggested Workflow

Based on the above, we recommend working in feature branches for each of your pull requests, all of them based off of your master and have drafted this recommended workflow:

# on branch master
git pull [email protected]:woce/LunaSysMgr # make sure that your master is up to date with upstream (WOCE) master
git checkout -b cards-magic-arrangement # create a branch called cards-magic-arrangement and check it out
# on branch card-mangic-arrangement
# change a few things
git add source/arranger_class_bar.h
git commit -m "Add new methods for the magic arranger to header file"
# change more stuff
git add source/arranger_class_bar.cpp
git commit -m "Make cards magically arrange"
git add source/magic_arranger_helper.cpp
git commit -m "add a helper class for the magic arranger"
# You're done with your feature, now let's get ready to pull request!
git pull [email protected]:woce/LunaSysMgr # this gets us back up to date with upstream master
# if there are any merge conflicts, solve them a la http://genomewiki.ucsc.edu/index.php/Resolving_merge_conflicts_in_Git
git push origin card-magic-arranger

Helpful Tip

You can use git add --patch or git add -p (especially its command s (which stands for split) for commiting partial patches. This allows clean commits without git stashes.

You now have a branch on your github fork which you can pull request in to WOCE master or WOCE features. When you pull request to the Github fork, you should see only the following commits (obviously they only apply to this example):

  • Add new methods for the magic arranger to header file
  • Make cards magically arrange
  • add a helper class for the magic arranger
  • Merge branch master of [email protected]:woce/LunaSysMgr

The last commit will only be shown if there was something to merge when you make that last git pull before pushing.

Tab Characters

All edits to C/++/JS webOS Ports projects should be made using tab characters for indentation, not spaces.

Most text editors provide a setting for this, and vim users can :set the following options:

 noet
 ts=4
 sw=4

Additionally, you can convert an entire file to using tab characters in vim by running the following:

 :set noet ts=4 |%retab!

Commits

Branch Layout

Branches should have some degree of organization to them- it's not a good idea to work directly on top of your upstream tracking branch (webOS-ports/master in most cases). Ideally you shouldn't commit anything to it, and use it as a reference to base your other 'feature' branches on. That way you can git pull and rebase your feature branches on it in order to stay up-to-date.

Rebasing

Before issuing a pull request, developers should update their upstream/master tracking branch (webOS-ports/master in most cases) and then rebase their feature branch on it with the following commands:

 git checkout webOS-ports/master
 git pull
 git checkout <yourbranch>
 git rebase <yourbranch> webOS-ports/master
 <fix any merge conflicts and commit>
 git push origin <yourbranch>

Example:

 git checkout webOS-ports/master
 git pull
 git checkout shiftyaxel/ui-scaling
 git rebase shiftyaxel/ui-scaling webOS-ports/master
 <fix any merge conflicts and commit>
 git push origin shiftyaxel/ui-scaling

This will make sure that your branch can be merged with ours without any conflicts, because you will have fixed them before issuing the Pull Request.

Once you have run the above commands, your Github repo page will show your feature branch under 'recently pushed' with a button to initiate a Pull Request.

Reworking

Sometimes a Pull Request won't quite work with what webOS Ports has in mind, and you'll be asked to rework it in some way. When doing this, you can still use your feature branch- there's no need to close the pull request and reopen a new one.

Any changes pushed to your feature branch will show in the existing pull request, where you can continue the discussion with the team and move towards getting your Pull Request approved.


The Contribution Process

For big changes, the contributor communicates with the Project via mailing lists or Bug Tracker tickets to get feedback before submitting code

  • Contributor adds signoff line to each commit message during development
  • Contributor makes a GitHub pull request (see https://help.github.com/articles/creating-a-pull-request)
  • Maintainer conducts code review, verifies signoff, runs tests and asks for adjustments from contributor as necessary
  • Maintainer merges the commits into the repo and closes the pull request

Criteria for Making a Pull Request

  • Contributor has verified that their changes do not break any of the builds
  • Contributor has provided or updated unit tests, if there is an existing unit test structure for any of the components affected
  • If there is no unit test structure, the contributor has thoroughly tested their changes manually, and can describe the results
  • Code is in the style of the code that surrounds it
  • Contributor has followed the commit guidelines

During Review the Maintainer Will:

  • Look to see that your signoff is in all your commit messages, including format, and presence of real name and real email address
  • If you are someone entirely new to the Project, they may get in touch with you via the contact information you have provided
  • If there are anomalies such as inconsistent name or email address between signoffs, they may ask you to clarify

This process may take some time, since we may conduct testing, and there may be concurrent activity which must be checked for merge conflicts, architectural issues, etc.

What You Will See Once Your Pull Request Has Been Reviewed and Accepted:

  • Your commits will be merged onto the master branch, with your SHA-protected signoff included
  • The maintainer's identity who accepted your pull request will be recorded in the merge
  • The pull request will be closed

Congrats, your contribution is in!

If Your Existing Pull Request Contains Commits that Don't Have the Signoff:

One way you can fix this:

Assuming no one has forked from your fork, or you don't mind breaking them use "rebase -i" to edit the existing commit messages (Google it) Open a new pull request with the fixed commits Close the existing pull request, with a comment showing the new one that it has superseeded.

Including changes in build

After a pull request has been closed and committed to the branch, it still needs to be updated in the recipe as well, so it's included in the future build.

Recipes can be found at https://github.com/webOS-ports/meta-webos-ports/ we're currently using the Dora branch which resides at https://github.com/webOS-ports/meta-webos-ports/tree/dora/

For the apps, the recipes can be found at https://github.com/webOS-ports/meta-webos-ports/tree/dora/recipes-webos/apps

There are different kinds of recipes

Recipes with SRCREV

These recipes are the most straight forward. Just update the SRCREV tag in the recipe with the SHA value of the respective commit for the app/component.

For example:

 SRCREV = "c8500b2040e0c8deceeaba369a04a4ba47e82d9c"

in https://github.com/webOS-ports/meta-webos-ports/blob/dora/recipes-webos/apps/org.webosinternals.preware.bb refers to commit https://github.com/webOS-ports/preware/commit/c8500b2040e0c8deceeaba369a04a4ba47e82d9c

Updating it to commit f69ab1b282eb77a11d4397e03575790871c755bb can be done by simply replacing

 SRCREV = "c8500b2040e0c8deceeaba369a04a4ba47e82d9c"

with

 SRCREV = "f69ab1b282eb77a11d4397e03575790871c755bb"

Recipes with WEBOS_VERSION

These recipes are a bit more tricky since they include release, submission as well as SHA.

For example in https://github.com/webOS-ports/meta-webos-ports/blob/dora/recipes-webos/apps/org.webosports.app.phone.bb

 WEBOS_VERSION = "0.1.0-3_6f4262951247971b3857ab95c04470f130cb18e0"

0.1.0 refers to release, 3 refers to submission and 6f4262951247971b3857ab95c04470f130cb18e0 refers to the SHA for the submission tag.

All these 3 values need updating in case of a new release or submission.

A submission always corresponds to a tag in the git repository in the format "submission/<number>". To create a new submission tag for a specific commit or just the HEAD of the git repository use the following command and replace <number> and <commit rev>:

 git tag submissions/<number> -m "submissions/<number>" <commit rev>

Don't forget to push the tag afterwards

 git push origin submissions/<number>

After the submission tag is created you can show its rev with the following command

 git show-ref submissions/<number>

The returned SRCREV is what has to be added to the WEBOS_VERSION variable as last part.

NOTE: If the build complains about the tag being on a commit on the wrong branch go and recreate the tag on the correct branch (don't forget to remove the tag before).

NOTE: After a tag is once submitted to a build the tag should never be touched again. The build will notice any modifications to the created tag and will fail!