(no commit message)
[openafs-wiki.git] / AFSLore / GitDevelopers.mdwn
index 248ebe9..16a9849 100644 (file)
@@ -2,7 +2,7 @@
 
 Git opens up a number of new options for contributing to OpenAFS. For the first time, it is easy to review code that is pending addition to the OpenAFS tree. In fact, reviewing code is one of the best ways to ensure that the releases that OpenAFS ships remain stable and functional. If you are interested purely in reviewing, then please skip to that section towards the end of this document.
 
-Git also changes the way that developers interact with the OpenAFS tree. Instead of just having a single version of the tree on your local machine, you have a compressed copy of the entire repository. Additionaly, you no longer have to produce patches to send code upstream - any developer can push into the OpenAFS repository directly, through gerrit, our code review tool.
+Git also changes the way that developers interact with the OpenAFS tree. Instead of just having a single version of the tree on your local machine, you have a compressed copy of the entire repository. Additionally, you no longer have to produce patches to send code upstream - any developer can push into the OpenAFS repository directly, through gerrit, our code review tool.
 
 Whilst git is a far more powerful tool than CVS it is also, inevitably, more complex. This document can only scratch the surface of what's possible with git - there are many, many, documents available that describe git in greater detail, and references to some of them are provided at the end.
 
@@ -36,7 +36,7 @@ will pull all of the new changes into your local repository, and merge those cha
 
 The OpenAFS repository contains many branches, most of which are historical and should not be used for new development. Current development should be targetted at 'master' (for feature work, and general bugfixing), or at 'openafs-stable-1\_4\_x' (bug fixes specific to the current stable release). Note that the openafs-devel-1\_5\_x branch is now effectively dead - future 1.5 releases will occur from the 'master' branch.
 
-A complete list of all branches can be obtained by running
+A complete list of all branches in the upstream OpenAFS repository can be obtained by running
 
     git branch -r
 
@@ -74,7 +74,7 @@ Again, whilst a direct checkout of a remote tag is fine for code browsing, it sh
 
 ## <a name="Viewing deltas"></a> Viewing deltas
 
-In git, a delta should be simply a single changeset. Deltas are represented by means of a special form of git tag, allowing you to locally view the change, and commit message, that corresponds to each one. In order to keep down the transfer size, deltas are not included in the repository you get when you do a git clone - there are over 10,000 delta references, and having them in your local repository can cause performance issues. If you really wish to be able to locally browse deltas, then run the following
+OpenAFS's original CVS repository used the concept of deltas as a means of grouping a large number of related changes into a single item, which could be easily fetched and referred to. In git, a delta should be simply a single commit. Deltas are represented by means of a special form of git tag, allowing you to locally view the change and commit message that corresponds to each one. In order to keep down the transfer size, deltas are not included in the repository you get when you do a git clone - there are over 10,000 delta references, and having them in your local repository can cause performance issues. If you really wish to be able to locally browse deltas, then run the following
 
     git config --add remote.origin.fetch '+refs/deltas/*:refs/remotes/deltas/*'
     git fetch origin
@@ -83,7 +83,7 @@ You can then view a specific delta by doing
 
     git show refs/remotes/deltas/<branch>/<delta>
 
-Sadly, historical accidents mean that not all of our deltas can be represented by means of single commit. Where this is the case, a delta-name will have a trailing -part-, where each of these numbers must be used to form the complete delta. This only applies to some deltas created before the git conversion - all deltas created from now on will be a single changeset.
+Sadly, historical accidents mean that not all of our deltas can be represented by means of single commit. Where this is the case, a delta-name will have a trailing -part-, where each of these numbers must be used to form the complete delta. This only applies to some deltas created before the git conversion - all deltas created from now on will be single commits.
 
 ## <a name="Introducing yourself to git"></a> Introducing yourself to git
 
@@ -99,6 +99,8 @@ If you want to make this settings for all of your repositories, then add the --g
 
 Note that this email address is the address by which you will be identified in [[OpenAFS]]'s revision history - it is also the address to which the gerrit code review tool will send all email related to the review of your code.
 
+If you plan on making changes to OpenAFS (and why else would you be reading this?) you should probably also grab <b>The change id hook</b> described in <b>Registering With gerrit</b> below. You can grab and apply the hook before registering, and it'll make sure your pre-registration development has the appropriate change IDs in the log. The hook only applies to your openafs development, so you're not going to mess up any of your non-OpenAFS work.
+
 ## <a name="Starting development"></a> Starting development
 
 We strongly recommend that you do all of your development upon 'topic branches' This allows you to isolate multiple unrelated changes, and makes it easier to keep your tree in sync with the upstream [[OpenAFS]] one.
@@ -107,7 +109,7 @@ Before creating a new topic branch, running
 
     git fetch
 
-will make sure that your repository is up to date (unlike git pull, this will not update any files that you may have checked out)
+will make sure that your repository knows about the latest upstream changes (unlike git pull, this will not update any files that you may have checked out)
 
 To create a new topic branch:
 
@@ -117,13 +119,13 @@ For example, to work on a patch to fix printf warnings, based on the current dev
 
     git checkout -b fix-printf-warnings origin/master
 
-This puts me on a new branch, ready to start writing code. We'd strongly recommend that all new development is based upon the origin/master branch, submissions based upon other branches are unlikely to be accepted.
+This puts me on a new branch, ready to start writing code. All new development should be based upon the origin/master branch, submissions based upon other branches are unlikely to be accepted, unless they address issues that are solely present in that branch.
 
-'git add' is used to tell git about any new files you create as part of your patch. If your patch results in any new compilation products (object files, new executables, etc) that git should not be tracking, please make sure that they're caught by the .gitignore mechanism. You can do this by checking that they don't appear in the output from 'git status'
+'git add' is used to tell git about any new files you create as part of your patch. If your patch results in any new compilation products (object files, new executables, etc) that git should not be tracking, please make sure that they're caught by the .gitignore mechanism. You can do this by checking that they don't appear in the output from 'git status' [Data Recovery Austin](http://www.securedatarecovery.com/data-recovery-austin.html)
 
 'git mv' and 'git rm' are used to move and delete files respectively.
 
-git commit is used to commit code. If you don't want to have to remember to 'git add' every time you change a file, then 'git commit -a' can be used to commit all changes to files that git knows about.
+'git commit -a' is used to commit code to all of the files that git is currently tracking (that is, all of the files that you have checked out from the repository, and all those which you have run git add on)
 
 ## <a name="When you can&#39;t see the wood for"></a><a name="When you can&#39;t see the wood for "></a> When you can't see the wood for the trees
 
@@ -141,7 +143,7 @@ will restore &lt;file&gt; to the state it was in at the last commit.
 
 If you're working on a long running development project, you will find that the point your created your topic branch rapidly recedes into history. At some point (and at least before you share your code with us), you'll probably want to update your tree. There are a number of ways of doing this.
 
-If you haven't shared you tree with anyone else, then you can use
+If you haven't shared your tree with anyone else, then you can use
 
     git rebase <branch> <topic>
 
@@ -149,7 +151,7 @@ If you haven't shared you tree with anyone else, then you can use
 
 Note that git rebase changes your local history (it moves the branch point of your topic branch to the tip of the upstream branch), and is a bad idea if others have cloned your repository. See 'man git-rebase' for more details.
 
-If you can't rebase, then consider either merging the changes onto your local branch, or creating a new topic branch and cherry picking your changes onto it. The man pages for 'git merge' and 'git cherry-pick' provide more detail on these.
+If you can't rebase, then consider either merging the changes onto your local branch, or creating a new topic branch and cherry picking your changes onto it. The man pages for 'git merge' and 'git cherry-pick' provide more detail on these options.
 
 ## <a name="Sharing your code with us"></a> Sharing your code with us
 
@@ -180,29 +182,39 @@ To make things easier, set up OpenSSH so that it knows about the defaults for th
 
 (where SSH Username is what you were told on the the 'SSH Keys' page)
 
+### <a name="The change id hook"></a> The change id hook
+
+Gerrit introduces the concept of "change IDs". This is a unique reference for a particular change, which remains constant regardless of any changes that are made to the implementation. This allows a single reference to be attached to a given modification, irrespective of any rewrites that may occur as a result of review comments. Manually maintaining change Ids is a pain, so gerrit provides a git hook which can be used to automatically add a change Id to any new modifications you create.
+
+The hook can be downloaded from the [[OpenAFS]] gerrit server by running the following, in the top level of your git tree
+
+    scp -p -P 29418 gerrit.openafs.org:hooks/commit-msg .git/hooks/
+
 ## <a name="Uploading to gerrit"></a> Uploading to gerrit
 
-When submitting to gerrit, it's important to realise that each commit in your branch will become a changeset in the upstream [[OpenAFS]], typically with no modification at our end. It is therefore important that these commits follow some simple rules...
+When submitting to gerrit, it's important to realise that each commit in your branch will become a changeset in the upstream OpenAFS, typically with no modification at our end. It is therefore important that these commits follow some simple rules...
 
 First, each commit should be complete. The maxim "one change per commit, one commit per change" applies here. Each commit should build and test in its own right. Typically, this means that a change will be in a small number of commits. If, during development, you have created many more than this (for example, you've created a large number of bug fix commits), please use 'git rebase', or cherry pick these commits into a separate tree before uploading them.
 
-Secondly, each commit should have a meaningful revision log. The internals of git means that we can't easily edit these before pushing them into the tree, so we'd like you to get them right for us! A commit message should be comprised of a single 'subject' line (which must **not** end with a full stop), followed by a blank line, followed by one or more paragraphs explaining the purpose of the patch. If it is intended to fix a bug in [[OpenAFS]] RT, then the word 'FIXES' followed by the bug number or comma-separated list of bug numbers should be included on a line of its own. The 'LICENSE' keyword can be used to indicate code which is covered under a license other than the IPL, although please speak to a gatekeeper if you intend using this. An example commit message would be
+Secondly, each commit should have a meaningful revision log. The internals of git means that we can't easily edit these before pushing them into the tree, so we'd like you to get them right for us! A commit message should be comprised of a single 'subject' line (which must **not** end with a full stop), followed by a blank line, followed by one or more paragraphs explaining the purpose of the patch. If it is intended to fix a bug in OpenAFS RT, then the word 'FIXES' followed by the bug number or comma-separated list of bug numbers should be included on a line of its own. The 'LICENSE' keyword can be used to indicate code which is covered under a license other than the IPL, although please speak to a gatekeeper if you intend using this. An example commit message would be
 
       Add option to disable syscall probing
 
       This adds the --disable-linux-syscall-probing to allow the probing of
-      kernel memory for the syscall table to be disable at compile time. This
+      kernel memory for the syscall table to be disabled at compile time. This
       helps with Linux architectures which have compile, run or load time
       issues with syscall probing by providing a band-aid fix until we can
       improve the configuration logic in this area.
 
       FIXES 123456
 
+Thirdly, each commit should have a valid changeID. Manually maintaining these is difficult and error prone, so we would strong advise that you install the changeID hook detailed earlier. This will automatically add a [[ChangeId]] line to your commit message if it doesn't already contain one.
+
 Once you have commits in this form, use
 
-    git log -p <branch>..HEAD
+    git log -p origin/<branch>..HEAD
 
-(where &lt;branch&gt; is the upstream branch upon which you are basing this patch)
+(where &lt;branch&gt; is the upstream branch upon which you are basing this patch).
 
 to check that what you're giving us makes sense. Then, upload them to gerrit using
 
@@ -223,8 +235,8 @@ Assuming all has gone well, this will have added the entry to the code review qu
 It's possible that your modifications won't be accepted first time. In this case, you need to revise your changes, and resubmit them to gerrit. Please note that this should always be done by modifying your original changeset, _not_ by submitting a new change that makes the required fixes. Either git commit --amend, or git rebase should be used to combine your changes with the original changeset, and then you should push this to gerrit with
 
     git push ssh://gerrit.openafs.org/openafs.git <hash>:refs/changes/<number>
-
-(where &lt;hash&gt; is the sha1 hash of the revised change, and &lt;number&gt; is the change number you received when you originally submitted the patch)
+(where &lt;hash&gt; is the sha1 hash of the revised change that follows the word <tt>commit</tt> in the log message, and &lt;number&gt; is the change number you received when you originally submitted the patch)
 
 You can obtain the sha1 hash of a commit by using 'git show' (if it is on the tip of your current branch), or 'git log' (if it is in your history)
 
@@ -258,15 +270,15 @@ You can send those into <openafs-bugs@openafs.org> as before. Note, however, by
 
 ## <a name="Reviewing changes"></a> Reviewing changes
 
-Okay, so that's how you get a patch into gerrit. Once it's there, then what happens to it? All code review happens via the <http://gerrit.openafs.org> interface. You should log in there as detailed above (using any OpenID), and make sure that the email address points to somewhere you'll read regularly.
+We'll now look at how changes that have made it into gerrit can be reviewed. All code review now happens via the <http://gerrit.openafs.org> interface. You should log in there as detailed above (using any OpenID), and make sure that the email address points to somewhere you'll read regularly.
 
 You'll be presented with a list of patches requiring review or, if someone has asked, patches you've been explicitly requested to review. There are two types of review - Code Review and Verification. Code Review means that you have read through the code, and are satisified that it works properly, follows the tree's style, and generally doesn't suck. Verification means that you have taken a copy of the patch and tested it. We hope to eventually automate the verification step, but for now both must be perfomed by hand.
 
-To perform a code review, go through each of the diffs in the current changeset for the code you have decided to review. You can double click on a line to leave a comment. Once you have completed commenting, click on the 'Publish Comments' button on the page containing the list of patch sets. You will then be asked to score the patch, with a range from -1 to +1. -1 means that you don't think the code should be applied, +1 means that it is good to apply. You can also leave further, general, comments for the patch submitter.
+To perform a code review, go through each of the diffs in the current changeset for the code you have decided to review. You can double click on a line to leave a comment. Once you have completed commenting, click on the 'Review' button that's about 3/4 of the way down the page containing the list of patch sets. You will then be asked to score the patch, with a range from -1 to +1. -1 means that you don't think the code should be applied, +1 means that it is good to apply. You can also leave further, general, comments for the patch submitter.
 
 Note that no matter how many +1 or -1 comments a patch receives, the gatekeepers can override these to either permit or forbid submission. Also, at least one gatekeeper must approve a patch before it can be submitted to the tree.
 
-To verify the code, pull the git copy into your local tree, using the git command listed as part of the patch details. For sanity's sake, we'd strongly recommend you do this pull into a branch you've created for the purpose. Build it, test it, and report your results. Note that a single -1 to verification can block patch submission, so please use these options wisely. If in doubt, score 0 and leave your comments. Please indicate when verifying which platforms you have tested on, and what testing you performed.
+To verify the code, pull the git copy into your local tree, using the git command listed as part of the patch details. For sanity's sake, we'd strongly recommend you do this pull into a topic branch you've created for the purpose. Build it, test it, and report your results. Note that a single -1 to verification can block patch submission, so please use these options wisely. If in doubt, score 0 and leave your comments. Please indicate when verifying which platforms you have tested on, and what testing you performed.
 
 And that's pretty much it. All of this is very new. If you encounter any problems at all, please ask for help on IRC in #openafs, Jabber in <openafs@conference.openafs.org> or on <openafs-devel@openafs.org>. Private pleas may be addressed to <simon@sxw.org.uk>
 
@@ -280,4 +292,8 @@ Git Community Book <http://book.git-scm.com/>
 
 Gerrit Documentation <http://gerrit.googlecode.com/svn/documentation/2.0/index.html> (only the first 'User Guide' section of this document is relevant)
 
+## <a name="Acknowledgments"></a> Acknowledgments
+
+Thanks to everyone who has reviewed this document, and offered corrections and contributions.
+
 -- [[SimonWilkinson]] - 07 Jul 2009