Difference between revisions of "Coding style/Version control practices"
| (5 intermediate revisions by one other user not shown) | |||
| Line 1: | Line 1: | ||
| − | The content on this page describes requirements and procedures which will apply after the master repository is converted from Subversion to git. Also, the content on this page has not been reviewed. |
||
| − | |||
The material on this page applies to developers with commit access to our master repository, or to contributors preparing contributions using a github fork. |
The material on this page applies to developers with commit access to our master repository, or to contributors preparing contributions using a github fork. |
||
== Commit messages == |
== Commit messages == |
||
| ⚫ | Begin each commit message with a summary line of less than 50 characters, capitalized but with no trailing period. |
||
| + | Generally, try to follow the advice in [http://chris.beams.io/posts/git-commit/ How to Write a Git Commit Message]. |
||
| + | |||
| ⚫ | Begin each commit message with a summary line of less than 50 characters, capitalized but with no trailing period. Think of this line like you would the subject of an email message. The summary should make sense without knowing what files are touched by the commit. Good summary lines make it easier to find commits in a gitk, github, or "git log --oneline" summary of commits. You should write the subject line in the imperative mood. |
||
If the change in the commit is not trivial, follow the summary line with a blank line and a longer description of the change. Do not explain every detail of the change, and do not use the commit message as a substitute for documenting the code. Instead, focus on explaining the motivation behind the change. Wrap lines at 70 characters. Here is an example: |
If the change in the commit is not trivial, follow the summary line with a blank line and a longer description of the change. Do not explain every detail of the change, and do not use the commit message as a substitute for documenting the code. Instead, focus on explaining the motivation behind the change. Wrap lines at 70 characters. Here is an example: |
||
| Line 17: | Line 17: | ||
</pre> |
</pre> |
||
| − | === RT headers === |
||
| + | Committers should also make sure to include the appropriate [[Committer_resources#RT_headers|RT headers]]. Contributors do not need to worry about them. |
||
| − | |||
| − | This subsection applies to committers only. Contributors can ignore it. |
||
| − | |||
| − | If a commit makes a user-visible change, such as adding a new feature or fixing a user-visible bug in a previous release, it should be associated with an RT ticket. Do this by adding RT pseudo-headers at the end of a ticket, separated by a blank line. The most important header is "ticket:", which associates the commit with a ticket number. Add "tags: pullup" and "target_version: X.Y.Z" if the commit should be pulled up to a release branch. |
||
| − | |||
| − | If you are creating a new ticket, run "ssh git.mit.edu krb5-rt-id" to reserve a ticket number, and put "ticket: NNNN (new)" in the log message, substituting the ticket number for NNNN. The summary line of the commit will be used as the subject of the new ticket; if this is not what you want, you can use a "subject:" RT header to set the ticket subject, or you can change the subject afterwards in RT. |
||
== One logical change per commit == |
== One logical change per commit == |
||
| Line 61: | Line 55: | ||
</pre> |
</pre> |
||
| − | You can use "git rebase -i" to cleanup a patch series |
+ | You can use "git rebase -i" to cleanup a patch series. |
== What to do if your push is rejected == |
== What to do if your push is rejected == |
||
Latest revision as of 15:22, 7 December 2015
The material on this page applies to developers with commit access to our master repository, or to contributors preparing contributions using a github fork.
Contents
Commit messages
Generally, try to follow the advice in How to Write a Git Commit Message.
Begin each commit message with a summary line of less than 50 characters, capitalized but with no trailing period. Think of this line like you would the subject of an email message. The summary should make sense without knowing what files are touched by the commit. Good summary lines make it easier to find commits in a gitk, github, or "git log --oneline" summary of commits. You should write the subject line in the imperative mood.
If the change in the commit is not trivial, follow the summary line with a blank line and a longer description of the change. Do not explain every detail of the change, and do not use the commit message as a substitute for documenting the code. Instead, focus on explaining the motivation behind the change. Wrap lines at 70 characters. Here is an example:
Make krb5_check_clockskew public
Rename krb5int_check_clockskew to krb5_check_clockskew and make it
public, in order to give kdcpreauth plugins a way to check timestamps
against the configured clock skew.
Committers should also make sure to include the appropriate RT headers. Contributors do not need to worry about them.
One logical change per commit
Do not combine several unrelated changes into a commit. Avoid the temptation to adjust nearby code formatting or re-word comments which are not related to the primary purpose of the commit. Although it's more work to split those changes out into separate commits, reviewing commits or examining history is easier when all of the changes in a commit have the same purpose.
However, it is okay to include test cases or documentation in the same commit as code changes. A bug fix could come in the same commit as a regression test for the bug, and a new feature could come in the same commit as its documentation and test cases.
Prepare a clean patch series
If you are working on a medium or large change which naturally breaks down into multiple logical changes, rewrite the history of your branch so that each commit makes one logical change and there are no development artifacts. For example, here are the summary lines of some recent ASN.1 work which was prepared as a patch series (most recent to least recent):
Remove unneeded ASN.1 code Convert utility functions to new decoder Data-driven ASN.1 decoder Change optional handling in ASN.1 encoder Style and naming changes to ASN.1 encoder Use size_t for lengths in ASN.1 encoder Minimize draft9 PKINIT code by removing dead code Eliminate some unused ASN.1 encoding primitives Fold atype_primitive into atype_fn Simplify ASN.1 choice type definitions Add ASN.1 decoder test for krb5_pa_pac_req
The team will not want to review a branch which looks like:
Add comments Refactor and cleanup Oops, add export symbols for client support Add server support Merge from trunk Rename frobnitz to whizzbang Add client support
You can use "git rebase -i" to cleanup a patch series.
What to do if your push is rejected
The master krb5 repository can be finicky about accepting commits. It will refuse pushes if they:
- try to delete a branch or tag.
- try to update a tag.
- try to rebase or revert a branch.
- try to create a ref which isn't a branch or tag.
- don't start each commit message with a summary line, or if the summary line is more than 50 characters, or if it ends with a period.
- start a commit message with something which looks like an RT header.
- put "ticket: new" in a commit message (this was the old way of creating a ticket in an SVN commit).
- add lines with trailing whitespace, or empty blank lines to the end of a file.
- put spaces before tabs in the indentation of a line.
- use tabs in a file which isn't supposed to have tabs (contains an emacs modeline with "indent-tabs-mode: nil").
- add a line at the end of a file which is missing a terminating newline.
The diagnostics for a failed push should report everything it found wrong. If you are trying to push a single commit, you can fix the problems by:
- Editing the changes to your working copy, if necessary
- Running "git commit --amend" (possibly with the -a option to include all edited files in the amended commit)
- Editing the log message
- Retrying the push
If you are trying to push several commits at once, you can use "git rebase -i HEAD~N" to modify any of the last N commits. git will bring up an editor window, in which you can change "pick" to "edit" for each of the commits you want to change. After you have closed the editor, git will give you a chance to perform edits and run "git commit --amend" for each commit you have chosen to edit, followed by "git rebase --continue". If at any point during this process you want to start over, you can run "git rebase --abort" to restore the head of the branch to what it was before you started the interactive rebase.
