View source for Version Control Systems 101

To help get everyone up to speed on the way we use VCS, specifically Git on projects like MPICH, we've put together this Wiki page as a reference. Some of the content here may be remedial, but please go through all of it to make sure you're contributing code in the way that everyone else on the team expects. This tutorial will cover:

* What Goes into Making Each VCS Commit?
* What Makes a Good Commit?
* Interoperation with Trac
* Git workflow (This is a short version of an excellent and much more extensive post ([[Git]])

I will assume that you're familiar with what a Version Control System is and why we use it. If you need a good introduction on VCS in general, you might try one of the many tutorials available on the web (citation needed).


== What Goes into Making Each VCS Commit? ==

[[File:commit_log.png | thumb | Sample Git Commit | 400px]]

# Header - This includes: 
#* The hash identifying the commit. We usually only use the first 8 characters as this is all that Trac looks at. You can get those by adding '''--abbrev-commit''' to your '''git log''' command.
#* The original author of the commit. This may be different than the person who originally wrote the commit if they do not have write privileges.
#* The date of the original commit. Again, this might be different than when it was actually pushed. 
# The Subject of the commit. This describes the actual changes in the commit and is critical to be both textually useful and correctly formatted. More on this later.
# The sign-off from another developer (if necessary). Anything that's not a trivial change (and sometimes even those) needs to be reviewed by another developer who is qualified to certify that your code is correct and formatted well.
# The code diff. This is the patch itself.


== What Makes a Good Commit? ==

[[File:single_idea.png | thumb | Single Idea Per Commit | 400px]]

* '''Only have a single idea per commit'''
** Often just a single file or a few lines.
** Makes automated testing much easier
*** If your commit includes features A & B and we have to roll it back to fix B, we don't want to lose A as well.
** Code refactoring should be separate from everything else.
*** If you need to change API calls, that should be completely separate.
* '''Don't pollute code that isn't yours''' - Try to make the least intrusive version of your change as possible. Don't try to fix things like whitespace in the same commit as code changes. If it's absolutely necessary to reformat existing code to make it more readable, that should be a separate commit.
** Doing this makes it hard to separate out who actually wrote the code.
* '''Format the code in the same style as the existing code'''
** If the existing code uses tabs instead of spaces, continue to use tabs. Again, if the code needs to be reformatted, make that a separate commit.
[[File:bad_whitespace.png | thumb | Bad Whitespace Example | 400px]]
* '''DO NOT INTRODUCE BAD WHITESPACE CHANGES'''
** This includes new whitespace at the end of the line or lines that only contain whitespace.
** Catch this early by turning on highlighting in your editor or using '''git show'''.
*** VIMRC
**** <code> autocmd ColorScheme * highlight ExtraWhitespace ctermbg=red guibg=red (this goes before colorscheme)<br> highlight ExtraWhitespace ctermbg=red guibg=red <br> match ExtraWhitespace /\s\+$/ <br> autocmd BufWinEnter * match ExtraWhitespace /\s\+$/ <br> autocmd InsertEnter * match ExtraWhitespace /\s\+\%#\@<!$/ <br> autocmd InsertLeave * match ExtraWhitespace /\s\+$/ <br> autocmd BufWinLeave * call clearmatches() <br> </code>
*** EMACS?
[[File:commit_message.png | thumb | Good Commit Message | 400px]]
* '''Write a good commit description'''
** The first line should be a short (<50 characters) description of the change.
*** Git uses this for the short version of the log
** Use a blank line between the first line and the rest of the description
** Write a complete description of the code change in the body.
** Use correct punctuation, spelling, and cApItaLizAtIOn
** [http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html Good blog post about how to do this correctly]
[[File:commit_trac.png | thumb | Commit Messages to Trac | 400px]]
* '''Refer to Trac tickets in your commit messages'''
** Using keywords makes Trac do magic to automatically attach commits to tickets
*** Fix #1234
*** Fixes #1234
*** Resolves #1234
*** Ticket #1234
*** See #1234
*** #1234

=== General Rules of MPICH Code ===

* Read and follow the [[Coding Standards]]
* Use 4 space indents, not tabs
* '''Don't leave extra whitespace!'''
* Comment everything that isn't completely obvious
* Wrap code text at 80 characters
** 72 for Git commit descriptions
** 50 for Git subject lines
* Use established naming conventions
* Check all return values!


== Using Git with MPICH ==

Git is huge, but this will cover the most important pieces for contributing to MPICH. Refer to the [[Git | wiki page]] for more details.

[[File:git_graph.png | thumb | Git graph with multiple repositories and branches | 400px]]
* Git is not SVN. It's not linear. It's a graph. Use it accordingly.
* Multiple repositories can intermingle with multiple branches.
** Often of an origin or mainline repository with a review queue in another repository
* Most changes shouldn't go into the main repository initially.
** They need to be reviewed by another developer who will sign off on them first.
** After being signed off, they can be pushed to the main MPICH repository.

=== MPICH Git Workflow ===

If you have commit privileges to mpich and mpich-review, this is how the workflow will be. If you don't have those rights, your workflow will probably include submitting patches or something else. If you are submitting patches, make sure you send '''git formatted''' patches which include the hash and original author so you will get credit and they are easy for us to apply. Note that for these instructions, we assume that you've followed the instructions in [[Git#Repository_Management]] to set up your repository (this means the main MPICH repository is named mpich rather than origin).

* Do work locally and commit.
** Each branch you work on should branch from mpich/master (not from another branch). This makes it easier to review as we can accept or reject each branch individually rather than as a whole. This also makes it easier to rebase your commits back onto mpich/master later when the commit is ready to go into the main repository.
** If your work is larger than one commit, you should create a new named branch to encompass the entire set of commits, then push this entire branch to mpich-review at once. You can also do this if your work is only one commit, but it's not required.
* Push to remote review repository
** <code> git push mpich-review HEAD:branch-name </code>
** If your code is longer than one commit, you may have to create a local branch where your work is staged, then push the entire branch to mpich-review.
* Someone else will review the code and sign-off (may require multiple signoffs)
** <code> git checkout mpich-review/branch-name </code>
** <code> git commit --amend --signoff </code>
** <code> git push -f mpich-review branch-name </code>
* Then it can be pushed to the main repository and the branch can be deleted
** <code> git push mpich branch-name:master </code>
** <code> git push mpich-review :branch-name </code>
*** This last step is important as it cleans up the branch in mpich-review. Once your commit is in the main mpich repository, there is no need to leave the old branch hanging around as it just makes everything more cluttered for the other developers.
** <code> git branch -D local_branch_name </code>
*** If you created a local branch, you can delete it at this point as it's been committed to the main repository. Note that you have to have some other branch checked out in order to delete this branch (you can't delete your current branch).


In general, we don't like to use merges in the MPICH repositories. Whenever possible, code should be rebased locally before being pushed into remote repositories. This makes it easier to follow the path of the code and simplifies code reviews. With a few exceptions, code should never be rebased when pushing to a remote repository as it changes the history of the code and makes synchronizing with other people who have checked out the code very difficult. The exception to this rule is with the mpich-review repository for those who have access. That code may be force pushed and history may be rewritten as it is part of our normal workflow.

=== Review Process ===

When your commit goes to be reviewed by someone else, the reviewer should look at these things:

* Does the code compile?
* Does it accomplish the goals set forth in the commit message?
* Can the code be meaningfully broken into multiple commits?
* Is the commit message useful and sufficient?
* Is there extra whitespace at the beginning/end of the line?
* Does this code meet the coding standards of the team?
* Is the formatting consistent with the existing code?
* Does this code introduce obvious new bugs?
* Does this code perform well (if applicable)?