[Cordova Wiki] Update of "CommitterWorkflow" by AndrewGrieve

[Apache Wiki <wi...@apache.org>]

Dear Wiki user,

You have subscribed to a wiki page or wiki category on "Cordova Wiki" for change notification.

The "CommitterWorkflow" page has been changed by AndrewGrieve:
https://wiki.apache.org/cordova/CommitterWorkflow?action=diff&rev1=24&rev2=25

  = First 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. Once you've chosen a password, test out your credentials with the following:
+ 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 checkout master
- git pull apache master
- git push apache master
+ git clone https://git-wip-us.apache.org/repos/asf/cordova-coho.git
+ 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 =
+ === Join the private mailing-list ===
  
  Send an email to private-subscribe@cordova.apache.org. Note that this is a moderated list, so your request to join must be manually accepted.
  
- = Do You Homework =
+ ===  Do You Homework ===
  
  Read through: [[http://www.apache.org/dev/committers.html]]
  
- = Committing Your Own Changes =
+ = Committing Changes =
  
- == Step 1: Mail the Mailing-list ==
+ === Step 1: Mail the Mailing-list (''optional'') ===
  This is required if any of the following are true:
   * Your change will add/remove/change any public Cordova APIs
   * You suspect that your change has a chance of being controversial
@@ -30, +40 @@

  
  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 [[http://www.apache.org/foundation/glossary.html#LazyConsensus|Lazy Consensus]].
  
- == Step 2: Ensure there is a JIRA issue. ==
+ === Step 2: Ensure there is a JIRA issue. ===
   * JIRA issues are used for both new features and for bugs.
-  * The "Fix For" field is used for the purpose of Release Notes.
-  * The issues are also used to track which commits / topic branches are related to them.
  
- == Step 3: Create a topic branch ==
+ === 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 ==
+ === Step 4: Make your changes ===
   * Thank you for making the world a better place.
+  * If appropriate, increase the repo's version number (see VersioningAndReleaseStrategy & StoringRepoVersionsDesign)
+  * If you're committing to a repo that uses `SemVer` (plugins, platforms, cli, plugman):
+    * If your commit adds a feature, then ensure the minor version is incremented (e.g. "3.0.1-dev"-> "3.1.0-dev")
+    * If your commit makes an non-backwards-compatible change, then ensure the major version is incremented (e.g. "3.0.1-dev"-> "4.0.0-dev")
  
- == Step 5: Test your changes ==
+ === Step 5: Test your changes ===
-  * The author is responsible to test their own changes and correct any problems with their changes before a pull request is submitted (contributor authored) or it lands in the stream (committer authored). The testing includes both verifying the function they added/touched, plus running the test suites to verify there are no regressions.
-  * When we say "run the test suites" this includes all automated tests in mobile-spec, manual tests in mobile-spec that might be affected by the change, and any platform-specific unit tests (i.e., cordova-android/test, cordova-ios/CordovaLibTests, cordova-js jake test, cordova-plugman npm test, etc.)
-  * It is recommended to add a short comment to the Jira item stating what testing you did.
-  * It is recommended that where reasonably feasible, you add automated tests to validate your change and catch any future regressions.
+  * You are responsible for testing the changes that push.
+  * This includes:
+    * All automated tests in mobile-spec
+    * Manual tests in mobile-spec that might be affected by the change
+    * Platform-specific unit tests (i.e., `cordova-android/test`, `cordova-ios/CordovaLibTests`, cordova-js: `grunt test`, cordova-plugman: `npm test`)
+  * If there is no existing test that exercises your code, add one!
   * If you are writing documentation (i.e., cordova-docs), be aware of the [[https://github.com/apache/cordova-docs/blob/master/STYLESHEET.md|style guidelines]].
  
- == Step 6: Ask for a code review ==
+ === Step 6: Ask for a code review (''optional'') ===
-  * If you are using a public topic branch, then you should ask for a code review when you consider it to be complete.
+  * Do this if you want a second pair of eyes to look at your code before it goes in.
   * Use [[http://reviews.apache.org/|reviews.apache.org]] to create a review request.
+    * Easiest way is to use type [[http://www.reviewboard.org/docs/rbtools/dev/|post-review]] from the repo with the change.
     * 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.
  
- == Step 7: Merge your change ==
+ === Step 7: Push your change ===
-  * Once your topic branch is tested & working, it's time to merge it.
+  * Rebase & commit it to the appropriate branch (see "''Which Branch to Commit To''" below).
-  * Rebase & commit it to master. If it fixes a regression, then also cherry-pick it into the appropriate release branch.
+  * If it fixes a regression, then also cherry-pick it into the appropriate release branch.
-  * Here is an example workflow for committing a change:
+  * Here is an example workflow for committing a change when you've made it on a topic branch:
  
  {{{
+ git pull
- git checkout master
- git pull apache
  git checkout topic_branch
  git rebase master -i
  git checkout master
  git merge --ff-only topic_branch
- git push apache master
+ git push
  git branch -d topic_branch
- # Cherry-pick:
- git checkout 2.6.x
- git cherry-pick -x COMMIT_HASH  # the -x flag adds "cherry-picked from <commit>" to the commit messages
- git push apache 2.6.x
  }}}
  
- The rebase -i step is your chance to clean up the commit messages and to combine small commits when appropriate. For example:
+  * Here is an example workflow for committing a change when you've made it on master:
+ 
  {{{
+ git pull --rebase
+ git rebase origin/master -i
+ 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 apache 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: Implemented RockOn feature (CB-1234)
+ Commit A: [CB-1234] Implemented RockOn feature
- Commit B: Added tests for RockOn (CB-1234)
+ 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. Having a smaller number of commits when merging makes it easier for others to comprehend the diff commits, and also makes it easier to roll-back commits should the need arise. For JS commits, prefix the message with [platform] so that it's clear who should take interest in the commit. For all commits, be sure to include the JIRA issue number / URL.
+  * 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 cordova-js commits:
+    * Prefix the message with [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 /.
  
- === Caveats of Rebasing ===
+ === 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).
+  * Close the issue, and set the "Fix Version" field.
+    * The "Fix Version" field is used for the purpose of Release Notes, and should be set to the [[VersioningAndReleaseStrategy|CadVer]]
  
- It's important to note when rebasing that the commits must never have been pushed or pulled before. If they have been, they will be duplicated in the history. The code should survive just fine, but the history will be a confusing tangle, especially if this happens multiple times.
  
- Note also that the timestamp on a commit will be unchanged by a rebase, but that it will appear "out of order" in git log. This is because git log is not chronological order, but in order of the `parent` chain of the commits.
+ = Which Branch to Commit To =
  
- == Step 8: Update JIRA ==
+ === 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`
  
- Mark the relevant JIRA as fixed, and be sure to include the relevant commit IDs.
+ === Core Plugins: ===
+  * Commit all changes to branch: `dev`
+    * Since `plugman` currently installs from `master`, we can't use it for development.
+ 
+ === Plugman: ===
+  * Commit all changes to branch: `master`
+ 
+ === CLI: ===
+  * Commit all changes to branch: `master`
  
  
  = Processing Pull Requests =
  
+ See ProcessingPullRequests
- == Step 1: Review the change ==
-  * View the user's branch in github and request changes be made (if applicable) by adding comments in the web interface
-  * Verify that the contributor tested it, per the test guidelines above.
  
- == Step 2: Ensure that 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, direct them to [[http://www.apache.org/licenses/#clas]]
- 
- == Step 3: Merging the change ==
- Run the following:
- {{{#!sh
- git remote add foo git://github.com/TheirUserName/cordova-docs.git
- git fetch foo
- git cherry-pick 5d3e1b6 # For each commit ID in their branch.
- git rebase origin/master -i
- }}}
- 
- The last 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-####]
- 
- 
- == 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
- }}}
- 
- If the committer is NOT set to YOU, and you'd like to reset it to yourself, well, not sure. I think the idea would be to do an extra merge, maybe on a new branch, probably with --no-ff (preventing a fast-forward), and then a commit --amend from there if the author got reset to you.
- 
- {{{
- git commit --amend --author=SomeAuthorIdHere
- }}}
- 
- 
- == Step 5: Push the change ==
- 
- {{{
- git push apache master
- }}}
- 
- == Step 6: Final details ==
- 
-  * Update related JIRA issue with the commit ID and close it if appropriate.
-  * Respond to pull request on github with link to their commit and ask them to close the pull request.
-