Difference between revisions of "Commit Guidelines"
Salty-horse (talk | contribs) (→Rules: Describe ALL and JANITORIAL) |
Salty-horse (talk | contribs) (→Rules: Tweak ALL description per LordHoto's suggestion) |
||
Line 37: | Line 37: | ||
* '''TOOLS''': ScummVM tools module or built-in tools | * '''TOOLS''': ScummVM tools module or built-in tools | ||
* '''MIDI''': MIDI sound-related | * '''MIDI''': MIDI sound-related | ||
* '''ALL''': A change that covers all subsystems | * '''ALL''': A change that covers multiple, or all, subsystems | ||
* '''JANITORIAL''': Cross-subsystem cleanups and formatting changes | * '''JANITORIAL''': Cross-subsystem cleanups and formatting changes | ||
Revision as of 20:35, 22 March 2012
In the past, ScummVM was quite lax on commit message formatting. Well, we won't start beating people up, but everyone should at least all be aware of the issue at hand; namely, what distinguishes good commit messages from bad ones. This page hopes to help all of us to improve our commit messages a bit... The benefit will be to the full team as commit logs will be more readable.
Rules
Thus the rules are following (they're standard to git except SUBSYSTEM thing):
SUBSYSTEM: Short (50 chars or less) summary of changes More detailed explanatory text, if necessary. Wrap it to about 72 characters or so. In some contexts, the first line is treated as the subject of an email and the rest of the text as the body. The blank line separating the summary from the body is critical (unless you omit the body entirely); tools like rebase can get confused if you run the two together. Write your commit message in the present tense: "Fix bug" and not "Fixed bug." This convention matches up with commit messages generated by commands like git merge and git revert. Further paragraphs come after blank lines. - Bullet points are okay, too - Typically a hyphen or asterisk is used for the bullet, preceded by a single space, with blank lines in between, but conventions vary here - Use a hanging indent
Particularly critical is the first line: This first line is used in short log format, and providing engine name, sound subsystem, backend name or whatever lets decide quickly should reviewers look deeply into commit or skip.
The subsystem names which were used so far:
- AGOS, SCI, SCUMM, whatever engine name
- SDL, Android, WII, NDS, whatever backend name
- OSystem: our middleware code
- i18n: internationalization
- GUI: all gui-related
- TOOLS: ScummVM tools module or built-in tools
- MIDI: MIDI sound-related
- ALL: A change that covers multiple, or all, subsystems
- JANITORIAL: Cross-subsystem cleanups and formatting changes
After first line, an empty line is required which is used as separator. This first line has to be short and less than 50 characters so the log will fit on a standard 80-column terminal.
Good Practice
Commit messages like the following aren't very helpful:
- "not needed" -> uhm, what is not needed?
- "bye bye" -> was some obsolete code removed? Or did somebody leave the project? Or what?
- "Reverse to match values" -> Reverse which values to match what?
- "Move to supported games" -> move what?
- "Forgot *.xpm files" -> forgot what about them?
- "One more file to fix"
- "This difference only applies to SCUMM7+" -> which difference?
- "" -> empty commit messages are about the worst you can do :-/ (short of insults and totally offtopic messages).
Always keep in mind -- those message are often read *without* seeing the diffs, and without the possibility to see which other files you committed just before that particular commit!
Compare this to
- "Change CVS keywords to SVN keywords"
- "Move Pajama3 to supported games"
- "Added compression tool for kyra speech files."
- "Add patch #1374870 - New Lure of the Temptress module"
Don't be too verbose in your message either. You don't have to tell people what the next step in the grand scheme is in a long paragraph.
More to read
http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
http://kernel.org/pub/software/scm/git/docs/user-manual.html#creating-good-commit-messages
The following is an excerpt from the FreeBSD Committer's Guide <http://www.freebsd.org/doc/en_US.ISO8859-1/articles/committers-guide/article.html> and sums up quite nicely what I think:
Excerpt from the FreeBSD Committer's Guide
Good commit messages are important. They tell others why you did the changes you did, not just right here and now, but months or years from now when someone wonders why some seemingly illogical or inefficient piece of code snuck into your source file. It is also an invaluable aid to deciding which changes to MFC and which not to MFC.
- Commit messages should be clear, concise and provide a reasonable summary to give an indication of what was changed and why.
- Commit messages should provide enough information to enable a third party to decide if the change is relevant to them and if they need to read the change itself.
- Avoid committing several unrelated changes in one go. It makes merging difficult, and also makes it harder to determine which change is the culprit if a bug crops up.
- Avoid committing style or whitespace fixes and functionality fixes in one go. It makes merging difficult, and also makes it harder to understand just what functional changes were made. In the case of documentation files, it can make the job of the translation teams more complicated, as it becomes difficult for them to determine exactly what content changes need to be translated.
- Avoid committing changes to multiple files in one go with a generic, vague message. Instead, commit each file (or small, related groups of files) with tailored commit messages.
Autoprops for subversion
Insert these in ~/.subversion/config
<syntax type="apache">
- Graphic file formats, usually binary:
- .bmp = svn:mime-type=image/bmp
- .gif = svn:mime-type=image/gif
- .ico = svn:mime-type=image/x-icon
- .jpeg = svn:mime-type=image/jpeg
- .jpg = svn:mime-type=image/jpeg
- .pdf = svn:mime-type=application/pdf
- .png = svn:mime-type=image/png
- .tiff = svn:mime-type=image/tiff
- .psd = svn:mime-type=image/x-photoshop
- Text / documentation files
- .rtf = svn:mime-type=text/rtf;svn:eol-style=native
- .tex = svn:mime-type=text/plain;svn:eol-style=native
README* = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id INSTALL = svn:mime-type=text/plain;svn:eol-style=native COPY* = svn:mime-type=text/plain;svn:eol-style=native HISTORY = svn:mime-type=text/plain;svn:eol-style=native
- .1 = svn:mime-type=text/plain;svn:eol-style=native
- .pc = svn:mime-type=text/plain;svn:eol-style=native
CHANGELOG = svn:mime-type=text/plain;svn:eol-style=native FILEFORMAT = svn:mime-type=text/plain;svn:eol-style=native THANKS = svn:mime-type=text/plain;svn:eol-style=native luac = svn:mime-type=text/plain;svn:eol-style=native
- Project / build files
- .bat = svn:mime-type=text/plain;svn:eol-style=native
- .mak = svn:mime-type=text/plain;svn:eol-style=native
- .mk = svn:mime-type=text/plain;svn:eol-style=native
- .dsp = svn:mime-type=text/plain;svn:eol-style=CRLF
- .dsw = svn:mime-type=text/plain;svn:eol-style=CRLF
Makefile* = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id project.pbxproj = svn:mime-type=text/xml;svn:eol-style=native
- Source code (keyword expansion enabled by default)
- .c = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .cpp = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .css = svn:mime-type=text/css;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .gap = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .h = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .hpp = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .htm = svn:mime-type=text/html;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .html = svn:mime-type=text/html;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .m = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .mm = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .php = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .inc = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .csv = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .js = svn:mime-type=text/javascript;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .tpl = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .po = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .pot = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
POTFILES = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .sed = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
po2c = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id;svn:executable=1
- .bdf = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .vcproj = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .sln = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .bat = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- .lua = svn:mime-type=text/plain;svn:eol-style=native;svn:keywords=Date Rev Author URL Id
- Various resource file formats
- .rc = svn:mime-type=text/plain;svn:eol-style=native
- .sh = svn:mime-type=text/plain;svn:eol-style=native;svn:executable
- .spec = svn:mime-type=text/plain;svn:eol-style=native
- .strings = svn:mime-type=text/plain;svn:eol-style=native
- .txt = svn:mime-type=text/plain;svn:eol-style=native
- .xhtml = svn:mime-type=application/xhtml+xml;svn:eol-style=native
- .xml = svn:mime-type=text/xml;svn:eol-style=native
- .xsl = svn:mime-type=text/xml;svn:eol-style=native
classes.nib = svn:mime-type=text/plain;svn:eol-style=native info.nib = svn:mime-type=text/plain;svn:eol-style=native objects.nib = svn:mime-type=application/octet-stream keyedobjects.nib = svn:mime-type=application/octet-stream
- .fcc = svn:mime-type=application/octet-stream
</syntax>