Repository: cordova-coho Updated Branches: refs/heads/master f7f027fad -> 73c5cdc06
Move from wiki: CommitterWorkfow and ProcessingPullRequests Project: http://git-wip-us.apache.org/repos/asf/cordova-coho/repo Commit: http://git-wip-us.apache.org/repos/asf/cordova-coho/commit/8857f71f Tree: http://git-wip-us.apache.org/repos/asf/cordova-coho/tree/8857f71f Diff: http://git-wip-us.apache.org/repos/asf/cordova-coho/diff/8857f71f Branch: refs/heads/master Commit: 8857f71f65e4ba968f4bca850bb752040c9b9e37 Parents: f7f027f Author: Andrew Grieve <[email protected]> Authored: Wed Mar 12 22:57:28 2014 -0400 Committer: Andrew Grieve <[email protected]> Committed: Wed Mar 12 22:57:28 2014 -0400 ---------------------------------------------------------------------- docs/code-reviews.md | 22 +++++ docs/committer-workflow.md | 155 ++++++++++++++++++++++++++++++++++ docs/processing-pull-requests.md | 90 ++++++++++++++++++++ 3 files changed, 267 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cordova-coho/blob/8857f71f/docs/code-reviews.md ---------------------------------------------------------------------- diff --git a/docs/code-reviews.md b/docs/code-reviews.md new file mode 100644 index 0000000..0d1500e --- /dev/null +++ b/docs/code-reviews.md @@ -0,0 +1,22 @@ +# Code Reviews + +TODO: Review Board didn't really work out :(. Come up with something better. + + * Use [[http://reviews.apache.org/|reviews.apache.org]] to create a review request. + * You will need a Review Board userid and password. One can be requested from the site. + * By default, the ML will be notified of your review request. + * If you have someone in particular that you would like approval from, be sure to add them in "People" field of the review. + * To create your review request is to use [[http://www.reviewboard.org/docs/rbtools/dev/|post-review]] (RBTools) from the repo with the change, only if the repo contains the file '.reviewboardrc' + Currently the following repos contain .reviewboardrc: + cordova-coho + cordova-cli + cordova-android + cordova-ios + cordova-js + + * If you don't want to use post-review tool, then on the web site: + * Click "New Review Request" + * On your workstation do a git diff and pipe the output to a file. In the new request, select the git repo, the base dir where you did the diff, upload the diff file you created, and click Create. + * On the next web page, fill in the description text, the name of the Branch (i.e., "master"), the Bug (i.e., "CB-4960"), the Description and the Testing Done fields. If you want a review from a specific person, enter their userid / name in the People field. If you want input from the whole community, enter "cordova" in the Groups field. Click the Submit button. + * After you have received sufficient feedback, click the button to mark your review as Discarded or Submitted [to a stream]. + * If you don't want to use !ReviewBoard you can use Github Pull Request http://git-wip-us.apache.org/repos/asf/cordova-coho/blob/8857f71f/docs/committer-workflow.md ---------------------------------------------------------------------- diff --git a/docs/committer-workflow.md b/docs/committer-workflow.md new file mode 100644 index 0000000..3c9525b --- /dev/null +++ b/docs/committer-workflow.md @@ -0,0 +1,155 @@ +# Committer Workflow + +## One-time Steps + +Congratulations! You've gained the confidence of your fellow Cordova committers +and have been granted the ability to commit code directly, and to apply pull +requests from others. You should receive an email from our Apache mentor with +the details of how to setup your account. + +### Configure your git repos + +It's convenient to have the origin of your git repos to point to Apache's repos +(as opposed to your clone of them on github). The easiest way to do this is to +delete them and re-clone them using coho: + + git clone https://git-wip-us.apache.org/repos/asf/cordova-coho.git + cd cordova-coho + npm install + cd .. + cordova-coho/coho repo-clone -r plugins -r mobile-spec -r ... + +Test out your credentials with the following: + + git pull + git push + +If all goes well, git push should have asked you for your username and password, and an "Everything up-to-date" message should have been printed. + +### Join the private mailing-list + +This is a list that only committers can join. + +Send an email to `[email protected]`. + +Note that this is a moderated list, so your request to join must be manually accepted. + +### Do Your Homework + +Read through: http://www.apache.org/dev/committers.html + +# Commit Workflow + +### Step 1: Mail the Mailing-list (_optional_) +This is required if any of the following apply: + * Your change will add/remove/change a public Cordova API + * You suspect that your change has a chance of being controversial + * You would like feedback before you begin + +When possible, try to phrase things in the form of a proposal. If no one objects (within a workday or two), then consider yourself to have [lazy consensus](http://www.apache.org/foundation/glossary.html#LazyConsensus). + +### Step 2: Ensure there is a JIRA issue + * They are not necessary for *all* changes, (e.g. style nits, refactoring) + * They should always be used for new features and bugs + +### Step 3: Create a topic branch (_optional_) + * Using a public topic branch is necessary only when you would like to collaborate on the feature. + * For small changes, private topic branches are preferred. + * Note: You should never rebase a public topic branch! + +### Step 4: Make your changes + * Thank you for making the world a better place. + +### Step 5: Test your changes ### + * You are responsible for testing the commits you push. + * Tests vary by repo, but in general: + * Plugins: Automated tests in mobile-spec and/or manual tests in mobile spec + * Tools: run `npm test` from the project root + * Platforms: Native unit tests (i.e., `cordova-android/test`, `cordova-ios/CordovaLibTests`) + * Cordova JS: Run `grunt test` + * If there is no existing test that exercises your code, consider adding one + * If you are writing documentation (i.e., cordova-docs), be aware of the [style guidelines](https://github.com/apache/cordova-docs/blob/master/STYLESHEET.md|style guidelines). + +### Step 6: Ask for a code review (_optional_) + * Do this if you want a second pair of eyes to look at your code before it goes in. + * Use either [reviewboard](code-reviews.md) or a GitHub pull request. + +### Step 7: Push your change + * When possible, rebase & squash your commits + * Make sure you can figure out what your commit does by the first line of your commit discription. + * If it fixes a regression, then also cherry-pick it into the appropriate release branch. + * Here is an example workflow for committing a change when you've made it on a topic branch + + + git pull + git checkout topic_branch + git rebase master -i + git checkout master + git merge --ff-only topic_branch + git push + git branch -d topic_branch + + * Here is an example workflow for committing a change when you've made it on master: + + + git pull --rebase + git rebase origin/master -i # Squash & reword commit messages + git push + + * If you ever end up with a merge commit on master that you don't want: + + + git rebase origin/master + + * If you need to add your change to a release branch: + + + git checkout 2.9.x + git cherry-pick -x COMMIT_HASH # the -x flag adds "cherry-picked from <commit>" to the commit messages + git push origin 2.9.x + +The `git rebase -i` step is your chance to clean up the commit messages and to combine small commits when appropriate. For example: + + Commit A: CB-1234 Implemented RockOn feature + Commit B: CB-1234 Added tests for RockOn + Commit C: Fixed RockOn not working with empty strings + Commit D: Renamed RockOn to JustRock + Commit E: Refactor MainFeature to make use of JustRock. + + * In this case, it would be appropriate to combine commits A-D into a single commit, or at least commits A & C. + * Fewer commits are better because: + * Easier to comprehend the diff + * Makes changelog more concise + * Easier to roll-back commits should the need arise + * __NEVER REBASE A COMMIT THAT HAS BEEN PUSHED__ + * For all commits: + * Prefix with JIRA IDs: CB-1234 + * For commits to cordova-js or to plugins: + * Prefix the message with the affected_platform so that it's clear who should take interest in the commit + * e.g.: `CB-1234 android: Improved exec bridge by using strings instead of JSON` + * e.g.: `CB-1234 all: Fixed plugin loading paths that start with /` + +### Step 8: Update JIRA + * An Apache bot should have already added a comment to the issue with your commit ID (based on the CB-1234 being in the commit message). + * Click the "Resolve Issue" button + * Add a comment saying what version the commit is in (e.g. Fixed in 0.1.3-dev). + +# Which Branch to Commit To + +### Platforms, mobile-spec, cordova-js, cordova-docs: + * Commit all changes to branch: `master` + * If it fixes a regression, cherry-pick into the release branch (see CuttingReleases) + * e.g. To cherry pick the last commit from master: `git cherry-pick -x master` + +### Plugins + * Commit all changes to branch: `dev` + * Through '''Cordova 3.0''', `plugman` installed from `master`, which means we couldn't use `master` for development. + * Post '''Cordova 3.0''', `plugman` has support for a registry... + +### All other Repos: + * Commit all changes to branch: `master` + +# Processing Pull Requests # + +See [processing-pull-requests.md](processing-pull-requests.md) + http://git-wip-us.apache.org/repos/asf/cordova-coho/blob/8857f71f/docs/processing-pull-requests.md ---------------------------------------------------------------------- diff --git a/docs/processing-pull-requests.md b/docs/processing-pull-requests.md new file mode 100644 index 0000000..459ce82 --- /dev/null +++ b/docs/processing-pull-requests.md @@ -0,0 +1,90 @@ +# Processing Pull Requests + +## Prerequisites: + * Ensure you are familiar with [committer-workflow.md](committer-workflow.md) + +## Step 0: + * Find what requests need attention by looking a the GitHub page. + * To look at them in aggregate: + + + ./cordova-coho/coho list-pulls | tee pulls.list | less -R + + * To filter out those that you last commented on: + + + ./cordova-coho/coho list-pulls --hide-user=agrieve + + * To show only certain repos: + + + ./cordova-coho/coho list-pulls -r js -r android -r plugin-inappbrowser + +## Step 1: Review the change (part 1) + * Ensure that we actually want the change (if unsure, bring it up on the ML) + * If there is no JIRA issue for the change, create one + * Ensure the JIRA issue has a link to the pull request + * Ensure the pull request has a link to the JIRA issue + * View the user's branch in github and request changes be made (if applicable) by adding comments in the web interface + +## Step 2: Ensure they have signed the Contributor Agreement + * For trivial changes, this is not necessary + * Find their name on: https://people.apache.org/committer-index.html#unlistedclas + * If it is not there, respond with: + +_Thanks for the pull request. I've had a look at it and think it looks good. Before we can merge it though, you need to sign Apache's Contributor License Agreement (can be done online): http://www.apache.org/licenses/#clas_ + +## Step 3: Merge the change +Run the following as an exemplary way to merge to master: + + git pull https://github.com/GitHubUser/cordova-FOO BRANCH + git rebase origin/MASTER_OR_DEV -i + +The rebase step will let you interactively review the changes being made to master. You should: + + * Squash as many commits as is reasonable together. + * Re-write commit messages to include JIRA issue numbers in the form CB-#### (no "[]"'s, no ":") + * In the final commit message (if there are multiple), [tell GitHub to close the pull request](https://help.github.com/articles/closing-issues-via-commit-messages) + +Example: + + CB-6124 Make `cordova plugin remove` resilient to a missing plugin directory + + This breaks a couple prepare tests that Andrew deletes in the next commit. + + github: close #57 + +## Step 4: Check the author + +Git keeps track of the author of commits separately from the committer of the commit. + +Typically, both of these values are set to the person submitting the pull request. +After your pull/merge/rebase/whatever work, you may find the values have changed - or not. +What we would typically be looking for is the final commit to have YOU as the committer and the original author as the author. + +You can check these fields with the following command: + + git log --pretty=full + +If the author is set to YOU, and you'd like to reset it to the original author, you can amend the commit: + + git commit --amend --author=some_author_id_here + +## Step 5: Review the change (part 2) + * You are responsible for testing the changes that push. + * Refer to [committer-workflow.md](committer-workflow.md) for how to test each repo. + * If it would be appropriate to write a test + * Either write one yourself, or ask the author to do so. + * If you write one yourself, commit it in a follow-up (don't squash it with theirs) + +## Step 6: Push the change + + git push + +## Step 7: Update JIRA + * Same as you would as if you authored the change. + +## Step 8: Final details + * The commit will get attached to the pull request automatically from the "closes #" in the commit message. + * If you haven't done so already, thank them for their contribution via a pull request comment :). +
