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 git@github.com: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

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'.

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:

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 git@github.com: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:

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

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.