Coding style/Version control practices - Revision history

TomYu at 20:22, 7 December 2015

2015-12-07T20:22:53Z

Ghudson: I don't recommend stacked git any more

2014-07-02T20:56:09Z

I don't recommend stacked git any more

Ghudson at 20:51, 4 October 2012

2012-10-04T20:51:49Z

Ghudson: /* RT headers */

2012-06-08T16:37:08Z

‎RT headers

Ghudson: /* RT headers */

2012-06-08T16:08:30Z

‎RT headers

Ghudson at 17:00, 12 May 2012

2012-05-12T17:00:59Z

Ghudson at 22:15, 21 April 2012

2012-04-21T22:15:04Z

Ghudson at 20:51, 15 April 2012

2012-04-15T20:51:20Z

Ghudson: /* Prepare a clean patch series */

2012-04-14T16:34:48Z

‎Prepare a clean patch series

Ghudson: New page: The content on this page has not been reviewed and does not reflect the policy of the development team (yet). Also, the instructions about RT headers rely on a change to rt-cvsgate which ...

2012-04-14T16:34:24Z

New page: The content on this page has not been reviewed and does not reflect the policy of the development team (yet). Also, the instructions about RT headers rely on a change to rt-cvsgate which ...

New page

The content on this page has not been reviewed and does not reflect the policy of the development team (yet). Also, the instructions about RT headers rely on a change to rt-cvsgate which has not yet been deployed.

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 ==

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.

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:

<pre>
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.
</pre>

=== RT headers ===

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. The "subject:" header can be used to set the subject of a ticket, if it is not already set. Add "tags: pullup" and "target_version: X.Y.Z" if the commit should be pulled up to a previous branch.

(TODO: update when we determine how "ticket: new" will be handled after the git conversion. Also if we make it possible to set subjects on new tickets with the summary line.)

== 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:

<pre>
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
</pre>

The team will not want to review a branch which looks like:

<pre>
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
</pre>

You can use "git rebase -i" to cleanup a patch series; for a more advanced toolset, consider trying out [http://www.procode.org/stgit/ Stacked Git].

← Older revision		Revision as of 20:22, 7 December 2015
Line 3:		Line 3:
	== Commit messages ==		== Commit messages ==

⚫	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.
		+	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 53: / Line 53: @@
 </pre>
-You can use "git rebase -i" to cleanup a patch series; for a more advanced toolset, consider trying out [http://www.procode.org/stgit/ Stacked Git].
+You can use "git rebase -i" to cleanup a patch series.
 == What to do if your push is rejected ==

← Older revision		Revision as of 20:51, 4 October 2012
Line 15:		Line 15:
	</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 the commit message, after 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. Here is an example:
−
−	Use correct profile var in krb5_get_tgs_ktypes
−
−	In r21879, when we converted to using KRB5_CONF macros for profile
−	variable names, we made a typo in krb5_get_tgs_ktypes and erroneously
−	started using default_tkt_enctypes instead of default_tgs_enctypes for
−	TGS requests. Fix the typo and return to the documented behavior.
−
−	ticket: 7155
−	target_version: 1.10.3
−	tags: pullup
−
−	A ticket can only have one target_version, so if a fix needs to go into multiple releases, only list the most recent release in the target_version header, and then make a comment in the RT ticket (via the RT web interface) that the ticket should also be pulled up to the other releases.
−
−	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 commit 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 31: / Line 31: @@
     target_version: 1.10.3
     tags: pullup
 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 commit 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.

@@ Line 19: / Line 19: @@
 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 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 the commit message, after 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.  Here is an example:
 == One logical change per commit ==

← Older revision		Revision as of 17:00, 12 May 2012
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.

@@ Line 1: / Line 1: @@
 The material on this page applies to developers with commit access to our master repository, or to contributors preparing contributions using a github fork.
@@ Line 21: / Line 21: @@
 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.  The "subject:" header can be used to set the subject of a ticket, if it is not already set.  Add "tags: pullup" and "target_version: X.Y.Z" if the commit should be pulled up to a previous branch.
+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.
 == One logical change per commit ==
@@ Line 62: / Line 62: @@
 You can use "git rebase -i" to cleanup a patch series; for a more advanced toolset, consider trying out [http://www.procode.org/stgit/ Stacked Git].

← Older revision		Revision as of 20:51, 15 April 2012
Line 1:		Line 1:
−	The content on this page has not been reviewed and does not reflect the policy of the development team (yet)~~. Also, the instructions about RT headers rely on a change to rt-cvsgate which has not yet been deployed~~.	+	The content on this page has not been reviewed and does not reflect the policy of the development team (yet).

	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.

@@ Line 33: / Line 33: @@
 == 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:
+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):
 <pre>