Difference between revisions of "Commit message guidelines"

From Octave
Jump to navigation Jump to search
(→‎Body of the commit message: mention avoiding patterns or abbreviations)
 
(5 intermediate revisions by 3 users not shown)
Line 4: Line 4:
  
 
   hg log --style changelog
 
   hg log --style changelog
 +
 +
*Note that this command will print all changelogs to the screen, currently including all changelogs back to 2008 and approaching 200,0000 lines of text. You may use the followig command for a paged output:
 +
 +
  hg log --style changelog | less
 +
 +
*Alternatively, you may save the changlogs to a text file. This will permit viewing and searching in a text editor for use in preparing your own commit messages:
 +
 +
  hg log --style changelog >> octave_changelogs.log
 +
  
 
== Guidelines ==
 
== Guidelines ==
 +
 
General structure of a commit message:
 
General structure of a commit message:
 +
 
:: '''One-line description'''
 
:: '''One-line description'''
:: Empty line
+
:: ''Empty line''
 
:: '''Body of the commit message'''
 
:: '''Body of the commit message'''
  
Each part is explained in detail below.  
+
=== One-line description ===
Here there is an example:
+
 
 +
The commit message should start with a brief one-line description of what the
 +
commit does. Keep it short, no longer than 80 characters. If you are working
 +
on a bug or applying a patch, this one-line explanation should mention the bug
 +
or patch number at the end like so: {{codeline|... (bug #12345)}}. Do not end
 +
the first line with a period (full stop).
 +
 
 +
If your change only touches one file, then the name of that file can be the
 +
prefix of the one-line description. If it's a C++ or C file, the function or
 +
class that is being modified should be included in the parenthetical remark,
 +
as in the full body of the commit message.
 +
 
 +
In addition, there are a few prefixes for certain types of commits:
 +
 
 +
* maint: for reorganisation of the sources that do not change the source. Regular merge commits are a prominent example.
 +
* doc: for changes to the documentation.
 +
* build: for changes to the build system, for example autoconf or automake files.
 +
 
 +
If your change is small and only touches one file, then the one-line
 +
description may serve as the entire commit message.
 +
 
 +
=== Body of the commit message ===
 +
 
 +
If there is more than one file touched in different ways and the one-line
 +
description isn't enough to describe all changes, the commit message needs a
 +
full-body description.
 +
 
 +
Each individual file changed by a commmit must have its changes enumerated.
 +
For changes affecting specific C++ functions, each function name is listed in
 +
parentheses. For example
 +
 
 
<pre>
 
<pre>
Give message id to undefined function error
+
* file.cc (class1::function1): Add something.
 +
(function2, function3): Delete something else.
 +
</pre>
 +
 
 +
For changes affecting specific Octave built-ins, each built-in name is listed
 +
in parentheses with an "F" prefix, an implementation detail. For example
  
* pt-id.cc (tree_identifier::eval_undefined_error): used
+
<pre>
  ::error_with_id instead of ::error to report undefined functions
+
* data.cc (Fcolumns): Return columns.
  with id.
 
 
</pre>
 
</pre>
  
=== One-line description ===
+
When the same change is applied to a series of files, or to a set of functions
 +
in a single file, the file or function names may be grouped to shorten the
 +
commit message. For example:
  
The commit message has a one-line description. Keep it short, preferrably under 80 characters. If you're squashing a bug, the bug number you're squashing should be included in this one-line description, like so at the end: <tt>(bug #12345)</tt>. Omit final period (full stop).
+
<pre>
 +
* file1.cc, file2.cc, file3.cc, file4.cc: Include <sys/types.h>.
 +
* memory.cc (function1, function2, function3): Throw error if empty.
 +
</pre>
  
Sometimes the one-line description is sufficient, for small changes. In that case, we use the following prefixes to give a description of what the small change is:
+
Each line of the commit message body should also be kept under 80 columns. The
 +
GNU standards recommend starting a new line for each parenthesized function,
 +
but if the line is short enough, we often avoid an extra newline. For example
  
* maint: for reorganisation of the sources that do not change the source. Regular merge commits are a prominent example.
+
<pre>
* doc: for changes to the documentation.
+
* file.cc (function1): Add an option. (function2): Add another option.
 +
</pre>
  
If your change only touches one file, then the filename of that file should be the prefix of the one-line description. If it's a C++ or C file, the function or class that is being modified should be included in the parenthetical remark, as in the full body of the commit message.
+
Only the last file name component is typically needed, since most files have
 +
unique names across the entire repository. One notable exception are the
 +
{{codeline|module.mk}} files in every directory, they should include the
 +
complete directory and file name. For example
  
=== Body of the commit message ===
+
<pre>
 +
* doc/interpreter/module.mk (dist_man_MANS): Include foo.1 in the list.
 +
</pre>
  
If there is more than one file touched in different ways and the one-line description isn't enough to describe all changes, the commit message needs a full-body description.
+
Avoid abbreviating or using shell globs or patterns when listing the names of
 +
files affected by a change, even when they have the same name with different
 +
file extensions. For example
  
Individual files you touched get their changes separately enumerated. If there a particular C or C++ function you changed, the general format is
+
<pre>
 +
* oct-fftw.cc, oct-fftw.h (octave_fftw_version): New function.
 +
</pre>
  
    * file.cc (class1::function1): Make change foo.
+
For m-file and Fortran sources, the function name can be omitted if the file
      (class2::function2): Make change bar.
+
contains only one function. For changes outside of functions or classes, of
 +
course the parenthetical (function) or (class::function) specifiers can also
 +
be omitted.
  
Note the newline for describing the change to a different function. General GNU style guidelines can be followed here, so that similar changes to different files can be grouped.
+
=== Wording ===
  
For style points, please also write "New function" instead of "Added function" or "Return retval" instead of "Changed to return retval".
+
Please write "New function" instead of "Added function" or "Return retval"
 +
instead of "Changed to return retval".
  
And please, never "Fixed bug" or similar. That doesn't add any specific information about what was changed.
+
Never write "Fixed bug" or similar. That doesn't add any specific
 +
information about what was changed.
  
The changelog info in the commit message should say what changed, not why. If you need to explain why, that info should probably go in comments in the code.
+
The commit message should describe what was changed, not why it was changed.
 +
Any explanation for why a change is needed should appear as comments in the
 +
code, particularly if there is something that might not be obvious to someone
 +
reading it later.
  
For m-script and Fortran sources, the function name can be omitted if the m-script only contains one file. For changes outside of functions or classes, of course the parenthetical (function) or (class::function) specifiers can also be omitted.
 
  
 
== Examples ==
 
== Examples ==
  
* standard commit messages
+
<pre>
 +
look for methods before constructors
 +
 
 +
* symtab.cc (symbol_table::fcn_info::fcn_info_rep::find):
 +
Look for class methods before constructors, contrary to Matlab
 +
documentation.
 +
* test/ctor-vs-method: New directory of test classes.
 +
* test/test_ctor_vs_method.m: New file.
 +
* test/Makefile.am: Include ctor-vs-method/module.mk.
 +
(FCN_FILES): Include test_ctor_vs_method.m in the list.
 +
</pre>
 +
 
 
<pre>
 
<pre>
 
allow abbreviations for optimset and optimget (bug #38999)
 
allow abbreviations for optimset and optimget (bug #38999)
Line 60: Line 139:
 
  ambiguous abbreviations. New tests.
 
  ambiguous abbreviations. New tests.
 
</pre>
 
</pre>
 +
 
<pre>
 
<pre>
 
add format option to ticklabel (bug #34906)
 
add format option to ticklabel (bug #34906)
Line 67: Line 147:
 
* graphics.in.h: define set_xyzticklabel as external function
 
* graphics.in.h: define set_xyzticklabel as external function
 
</pre>
 
</pre>
 +
 
<pre>
 
<pre>
 
tag symbols in indexed assignments as variables (bug #39240)
 
tag symbols in indexed assignments as variables (bug #39240)
Line 75: Line 156:
 
</pre>
 
</pre>
  
* For short and very simple commit messages under 80 characters.  
+
<pre>
<pre>@ftp/cd.m: Fix docstring.</pre>
+
tar, untar, unpack: Add support for BSD tar (bug #53695)
<pre>strjoin.m: delimiter can include escape sequences (matlab compatibility)</pre>
+
 
 +
* tar_is_bsd.m: New function.
 +
* tar.m: Use it to determine how to run tar and parse command output.
 +
* unpack.m: Likewise.
 +
</pre>
 +
 
 +
=== One line commit examples ===
 +
 
 +
This examples are the rare cases where only one file is modified and the
 +
change is simple enough:
 +
 
 +
<pre>maint: merge away accidental head.</pre>
 +
<pre>maint: Strip trailing whitespace from source files.</pre>
 +
<pre>maint: Update gnulib to latest changes.</pre>
 +
<pre>maint: Periodic merge of stable to default.</pre>
 +
<pre>doc: grammarcheck documentation for 4.2 release.</pre>
 +
<pre>pkg.m4: update to lastest version as released with pkg-config 0.29 (bug #48775)</pre>
 +
<pre>uigetfile.m: allow path names as input arg (bug #48828)</pre>
  
* maintenance or docs commits, no actual code changed
 
<pre>maint: update config.in.h in .hgignore</pre>
 
  
 
[[Category:Development]]
 
[[Category:Development]]

Latest revision as of 14:12, 14 May 2020

Our commit messages for Mercurial get automatically distilled into GNU Changelog entries. The GNU coding standards have some guidelines for how to write Changelogs, and since Octave is a GNU project, we try to produce Changelogs in this style. However, certain things have to be adapted because the style in there is primarily for C sources, and because we are producing them from Mercurial commit messages.

You can see how Mercurial will produce the Changelog-style output with the following command:

 hg log --style changelog
  • Note that this command will print all changelogs to the screen, currently including all changelogs back to 2008 and approaching 200,0000 lines of text. You may use the followig command for a paged output:
 hg log --style changelog | less
  • Alternatively, you may save the changlogs to a text file. This will permit viewing and searching in a text editor for use in preparing your own commit messages:
 hg log --style changelog >> octave_changelogs.log


Guidelines[edit]

General structure of a commit message:

One-line description
Empty line
Body of the commit message

One-line description[edit]

The commit message should start with a brief one-line description of what the commit does. Keep it short, no longer than 80 characters. If you are working on a bug or applying a patch, this one-line explanation should mention the bug or patch number at the end like so: ... (bug #12345). Do not end the first line with a period (full stop).

If your change only touches one file, then the name of that file can be the prefix of the one-line description. If it's a C++ or C file, the function or class that is being modified should be included in the parenthetical remark, as in the full body of the commit message.

In addition, there are a few prefixes for certain types of commits:

  • maint: for reorganisation of the sources that do not change the source. Regular merge commits are a prominent example.
  • doc: for changes to the documentation.
  • build: for changes to the build system, for example autoconf or automake files.

If your change is small and only touches one file, then the one-line description may serve as the entire commit message.

Body of the commit message[edit]

If there is more than one file touched in different ways and the one-line description isn't enough to describe all changes, the commit message needs a full-body description.

Each individual file changed by a commmit must have its changes enumerated. For changes affecting specific C++ functions, each function name is listed in parentheses. For example

* file.cc (class1::function1): Add something.
(function2, function3): Delete something else.

For changes affecting specific Octave built-ins, each built-in name is listed in parentheses with an "F" prefix, an implementation detail. For example

* data.cc (Fcolumns): Return columns.

When the same change is applied to a series of files, or to a set of functions in a single file, the file or function names may be grouped to shorten the commit message. For example:

* file1.cc, file2.cc, file3.cc, file4.cc: Include <sys/types.h>.
* memory.cc (function1, function2, function3): Throw error if empty.

Each line of the commit message body should also be kept under 80 columns. The GNU standards recommend starting a new line for each parenthesized function, but if the line is short enough, we often avoid an extra newline. For example

* file.cc (function1): Add an option.  (function2): Add another option.

Only the last file name component is typically needed, since most files have unique names across the entire repository. One notable exception are the module.mk files in every directory, they should include the complete directory and file name. For example

* doc/interpreter/module.mk (dist_man_MANS): Include foo.1 in the list.

Avoid abbreviating or using shell globs or patterns when listing the names of files affected by a change, even when they have the same name with different file extensions. For example

* oct-fftw.cc, oct-fftw.h (octave_fftw_version): New function.

For m-file and Fortran sources, the function name can be omitted if the file contains only one function. For changes outside of functions or classes, of course the parenthetical (function) or (class::function) specifiers can also be omitted.

Wording[edit]

Please write "New function" instead of "Added function" or "Return retval" instead of "Changed to return retval".

Never write "Fixed bug" or similar. That doesn't add any specific information about what was changed.

The commit message should describe what was changed, not why it was changed. Any explanation for why a change is needed should appear as comments in the code, particularly if there is something that might not be obvious to someone reading it later.


Examples[edit]

look for methods before constructors

* symtab.cc (symbol_table::fcn_info::fcn_info_rep::find):
Look for class methods before constructors, contrary to Matlab
documentation.
* test/ctor-vs-method: New directory of test classes.
* test/test_ctor_vs_method.m: New file.
* test/Makefile.am: Include ctor-vs-method/module.mk.
(FCN_FILES): Include test_ctor_vs_method.m in the list.
allow abbreviations for optimset and optimget (bug #38999)

* optimset.m, optimget.m: Handle abbreviated keys and warn for
 ambiguous abbreviations. New tests.
add format option to ticklabel (bug #34906)

* graphics.cc: add new functions to support different input arguments to
  xyzticklabel. Add tests.
* graphics.in.h: define set_xyzticklabel as external function
tag symbols in indexed assignments as variables (bug #39240)

* pt-arg-list.cc (tree_argument_list::variable_names): Also return the
  symbol names from index expressions.
* parser.tst: New test.
tar, untar, unpack: Add support for BSD tar (bug #53695)

* tar_is_bsd.m: New function.
* tar.m: Use it to determine how to run tar and parse command output.
* unpack.m: Likewise.

One line commit examples[edit]

This examples are the rare cases where only one file is modified and the change is simple enough:

maint: merge away accidental head.
maint: Strip trailing whitespace from source files.
maint: Update gnulib to latest changes.
maint: Periodic merge of stable to default.
doc: grammarcheck documentation for 4.2 release.
pkg.m4: update to lastest version as released with pkg-config 0.29 (bug #48775)
uigetfile.m: allow path names as input arg (bug #48828)