Commit Guidelines

This page contains some guidelines about commits and commit messages.

Commit guidelines

Commits shouldn't contain multiple unrelated changes; try and make piecemeal changes if you can, to make it easier to review and merge. In particular, don't commit style/whitespace changes and functionality changes in a single commit.

If you change shared common code, then (while not necessarily in the same commit, due to the above guideline!) you should also make at least a best-effort attempt to make sure all of the engine/backend code stays working.

Assure that ScummVM compiles with every commit. In case of regressions, this helps to track down the commit introducing the regression.

Changes to user facing elements and common code also requires proper documentation.

No merge commits are allowed, we enforce linear history.

Commit message formatting

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.

The rules on commit messages are the following (they're standard to git, except our SUBSYSTEM requirement):

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 a short log format, and providing engine name, sound subsystem, backend name, or whatever - lets us 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
BUILD: Files related to building the project, makefiles, and related
ALL: A change that covers multiple, or all, subsystems
JANITORIAL: Cross-subsystem cleanups and formatting changes

After the first line, an empty line is required which is used as the separator. This first line has to be short and less than 50 characters so the log will fit on a standard 80-column terminal.

Commit messages

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.

Documentation

Commits and PR should follow our documentation guidelines as part of the submission process. Documentation falls under two categories: User facing and Developer related

User facing

Any user facing changes must also have proper documentation. These commits fall under the DOCS subsystem. For information on how to commit and build our docs portal can be found at the Developer Central

Example of changes that require accompanying documentation:

GUI - New features, text changes of existing UI elements, addition of UI options / elements, Engine/Game specific settings
INI - Added keys and groups
Keymap - New keymaps, or change to existing keymaps
Ports - New ports, changes to documented ports
Features - Brand new features, for example Cloud Integration

Developer changes:

Any code contributions to common code outside of engines and backends must be accompanied with proper doxygen style comments. This includes but not limited to:

New methods
Signature changes
Changes to documented and undocumented methods
New classes

Documentation can be committed separately under the DOXYGEN subsystem. Code contributions in engines and backends do not have a hard requirement for doxygen style comments, but they are encouraged.

Commit Guidelines

Contents