Skip to content
This repository has been archived by the owner on Aug 5, 2022. It is now read-only.

Contributing

John Villalovos edited this page Feb 2, 2018 · 6 revisions

Contributing to ZJS

We use the three main communication methods during the development process:

  • GitHub pull requests to submit, review, and merge code changes
  • GitHub issues to file bugs, feature requests, and
  • IRC for day-to-day communications and collaboration

Let's take them in reverse order:

IRC

We communicate on FreeNode IRC on the channel #zjs. The core development team is located on the west coast of the US, so you're most likely to find us there roughly 9am-6pm Pacific time.

We're happy to help you there with any questions, configuration issues, debugging help, discussion, or whatever.

GitHub Issues

You're welcome to submit whatever you want as a GitHub issue, and we will generally try to make sure these things are resolved to your satisfaction before closing them. But we also want to keep our plate relatively clean so if it's not going to be addressed for a long time we may close it for that reason.

So far we've seen basically four types of issues: questions, RFCs making a proposal and looking for feedback, bug reports, and feature requests. We have labels to identify these types of issues but aren't particularly disciplined about applying them.

We use some other labels such as P1/P2/P3/P4 for priority, and self-explanatory ones like "needs rebase", "needs research", "reconsider later". These are evolving as we figure out what we need.

GitHub Pull Requests

Set up your fork and local codebase

To submit code, you should first create your own fork of our repo. Then clone your fork to your local machine. You might also want to set up the main repo as a remote named "upstream", like we do, since that can come in handy.

$ git remote add upstream [email protected]:intel/zephyr.js.git

We suggest you keep your master branch as a clean mirror of the upstream master. To keep your fork up to date, you would periodically do something like this:

$ git checkout master        # check out your local master branch
$ git fetch upstream         # sync to the latest upstream code database
$ git merge upstream/master  # merge the latest upstream changes (should apply cleanly)
$ git push                   # update your remote GitHub fork with the latest

(There may be a simpler way to do this so feel free to edit or provide feedback.)

Start a new change

When you're ready to work on a code change, you create a topic branch locally. For example, after syncing to the latest master code as above, you could do this:

$ git checkout -b my-branch-name

Typically, you will be submitting your commit against master branch where main development occurs. In that case, you should have checked out the master branch before the above command. But you may be submitting a change for a particular release branch like zjs-0.2 and in that case you should start with that branch.

Initial submission of a change

Once you've made your changes and debugged them, check them in as one or more commits. Try to keep one git commit to one "idea", give or a take a few one-liner tweaks to whitespace or wording and such. I'd much prefer to see five 100-line commits that each make sense on their own than one 500-line commit. But I know that sometimes you end up developing a few related things together in the same patch and it's not always worth the effort to tease them apart.

A pull request can have multiple commits that are meant to be preserved, but unless they're slam-dunks that can make the review process a bit more challenging. So usually it's one commit per pull request, initially.

Commit messages

(Note: When making changes in response to a code review, we'd like you to follow different guidelines - see below.)

Each commit should have a one-line summary followed by a blank line, followed by as much detailed description as needed, followed by a Signed-off-by line with your name and email address. The summary line should start with a category description such as [gpio] in lowercase. Try to follow the conventions we've used for similar commits in the past. Your summary line should at most 64 characters to leave room for GitHub to add the issue number like (#501). (We'd like it to not wrap when using git log --online.)

Once you have your local topic branch the way you want it, push it up to your fork:

$ git push origin my-branch-name

Then you can visit your fork's home page on GitHub (e.g. https://github.com/yourid/zephyr.js) and you should see your new branch with a "Compare and pull request" button. If you're ready, click that to submit the pull request.

Travis CI testing

Each time you submit or update a pull request, some tests will be run on it using Travis. If you need to look into detail, the tests are controlled by a file .travis.yml in the repo root directory. If your commit fails these tests, you should rework it until they pass or (rarely) talk to us about why the tests need to change. It takes roughly 7 minutes for the tests to complete unless tests are running on other pull requests right at the same time.

If you do have failures, to make resolving them simpler we have a script trlite that can run the same tests on your local machine. This is faster and there are command line arguments to let you narrow it down to the tests that are failing to make it even faster. Run trlite -h to see how to call it but just trlite will run the full battery.

We will typically not bother reviewing your patch until it's passing Travis.

Review process

We should notice your pull request within a few days and start to take a look, unless we're really swamped by a rebase effort or something. If it's not getting noticed, your best bet is to join the IRC channel and talk to us about it. But you can also try just submitting another comment in the PR and maybe calling one of us out by name (like @grgustaf).

When we request changes, try to resolve them and then submit them as a new commit. In this case, don't try to format your commit message carefully because we will squash the commit into the original one and discard this message. So just give a quick description like "fixed race condition and whitespace issues". The purpose of this is to keep the new changes separate so that the reviewer can quickly see what has been updated and make sure it meets expectations rather than reviewing the whole thing again.

If your pull request is large or complex and takes a while to get in, you may need to rebase it at some point to address conflicts with changes that have gone in since you started. GitHub will inform you of this on your pull request page. That's a bit complex. If you need help with it, talk to us on IRC and maybe I'll add details here.

We also may ask you to rebase when there are no conflicts just to ensure it works with the latest code before we merge it. In this case, and if you know what you're doing, you might want to squash fixups from review comments that have already been accepted. (If your PR has multiple mergeable commits you may need to pick and choose where to squash them.)

Merging

Once your patch has been accepted reviewed by one or more core developers with a +1 or LGTM message, or "Approval" of a GitHub review, we will merge it. We prefer to keep our tree serialized, free of merge commits. So we either squash and merge your commits into one, or rebase them based on the latest code.

If your patch does include multiple commits with nicely formatted commit messages, we'll assume you want them to be preserved in the git history and we'll use the rebase method of merging. If you have any extra "fixup" commits between them from the reviewing process, we'll probably ask you to clean up your history using git rebase -i.

If there is only one clean commit messages, perhaps with a bunch of fixup commits on top of it, we'll merge them all together with the squash method of merging.