Skip to content

Commit

Permalink
Doc: Overhaul guidelines for commit and commit-messages.
Browse files Browse the repository at this point in the history
* Move "Guidelines for Commits" from quickstart to commit-messages,
  while reworking it.

* Split "Content of the commit message" into an intro, "the 1st line",
  and "the body", reworking it.
  • Loading branch information
htgoebel committed Apr 16, 2018
1 parent f4edeed commit 1f092e1
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 61 deletions.
163 changes: 138 additions & 25 deletions doc/development/commit-messages.rst
@@ -1,3 +1,76 @@

.. _Guidelines for Commits:

=============================
Guidelines for Commits
=============================

**Please help keeping code and changes comprehensible for years.
Provide a readable commit-history following this guideline.**

A commit

* stands alone as a single, complete, logical change,

* has a descriptive commit message (see :ref:`below <commit messages>`),

* has no extraneous modifications (whitespace changes,
fixing a typo in an unrelated file, etc.),

* follows established coding conventions (:pep:`8`) closely.

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.

If you did several unrelated changes before committing, ``git gui`` makes
committing selected parts and even selected lines easy. Try the context menu
within the windows diff area.

This results in a more readable history, which makes it easier to understand
why a change was made. In case of an issue, it's easier to `git bisect` to
find breaking changes any revert those breaking changes.


In Detail
====================

A commit should be one (and just one) logical unit.
It should be something that someone might want to patch or
revert in its entirety, and never piece-wise.
If it could be useful in pieces, make separate commits.

* Make small patches (i.e. work in consistent increments).

* Reformatting code without functional changes will generally not be
accepted (for rational see :issue:`2727`).
If such changes are required, separate it into a commit of it own
and document as such.

This means
that when looking at patches later, we don't have to wade through loads of
non-functional changes to get to the relevant parts of the patch.

* Especially don't mix different types of change, and put a standard prefix
for each type of change to identify it in your commit message.

* Abstain refactorings!
If any, restrict refactorings (that should not change functionality) to
their own commit (and document).

* Restrict functionality changes (bug fix or new feature) to their own
changelists (and document).

* If your commit-series includes any "fix up" commits
("Fix typo.", "Fix test.", "Remove commented code.")
please use ``git rebase -i …`` to clean them up
prior to submitting a pull-request.

* Use ``git rebase -i`` to sort, squash, and fixup commits
prior to submitting the pull-request.
Make it a readable history, easy to understand what you've done.


.. _commit messages:

===================================
Expand All @@ -7,6 +80,10 @@ Please Write Good Commit Messages
**Please help keeping code and changes comprehensible for years.
Write good commit messages following this guideline.**

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.

|PyInstaller| is maintained since 2005 and we often need to
comprehend years later why a certain change has been implemented as it is.
What seemed to be obvious when the change was applied may be just obscure
Expand All @@ -25,46 +102,80 @@ rebase -i …`` and ``git push -f …`` to update your pull-request. See
Content of the commit message
==================================

* Write your commit message in the present tense: "Fix bug" and not "Fixed
bug." Consider these messages as the instructions for what applying the
commit will do. Further this convention matches up with commit messages
generated by commands like git merge and git revert.

* If you find yourself describing implementation details, this most probably
should go into a source code comment.
**Write meaningful commit messages.**

* Please include motivation for the change, and contrasts its implementation
with previous behavior.
* The first line shall be a short sentence
that can stand alone as a short description of the change,
written in the present tense, and
prefixed with the :ref:`subsystem-name <commit message standard prefixes>`.
See :ref:`below <commit message first line>` for details.

* For more complicate or serious changes please document relevant decisions,
contrast them with other possibilities for chosen,
side-effect you experienced,
or other thinks to keep in mind when touching this peace of code again.
(Although the later *might* better go into a source code comment.)
* The body of the commit message should explain or justify the change,
see :ref:`below <commit message body>` for details.

Examples of good commit messages are
:commit:`5c1628e66e18e2bb1c44faa88387b1f627181b43` or
:commit:`73d7710613e26c3d59212e9e031f41a916c1e892`.


Format of the commit message
==================================
.. _commit message first line:

The first Line
=====================

* The first line of the commit message should be a short summary that can
stand alone as a short description of the change.
The first line of the commit message shall

* The first line should start with the "subsystem" this commit is related to,
see the :ref:`list of standard prefixes <commit message standard prefixes>`
below.
* be a short sentence (≤ 72 characters maximum, but shoot for ≤ 50),

* This may optionally be followed by a separating blank line and details in
whatever form the commenter likes.
* use the present tense ("Add awesome feature.") [#]_,

* Try to end summary lines with a period. Ending punctuation other than a
* be prefixed with an identifier for the
:ref:`subsystem <commit message standard prefixes>`
this commit is related to
("tests: Fix the frob." or "building: Make all nodes turn faster."),

* always end with a period.

* Ending punctuation other than a
period should be used to indicate that the summary line is incomplete and
continues after the separator; "..." is conventional.

* For best results, stay within 72 characters per line. Don't go over 80.
.. [#] Consider these messages as the instructions for what applying the
commit will do. Further this convention matches up with commit messages
generated by commands like git merge and git revert.
.. _commit message body:

The Commit-Message Body
==================================

The body of a commit log should:

* explain or justify the change,

- If you find yourself describing implementation details, this most probably
should go into a source code comment.

- Please include motivation for the change, and contrasts its
implementation with previous behavior.

- For more complicate or serious changes please document relevant decisions,
contrast them with other possibilities for chosen,
side-effect you experienced,
or other thinks to keep in mind when touching this peace of code again.
(Although the later *might* better go into a source code comment.)

* for a bug fix, provide a ticket number or link to the ticket,

* explain what changes were made at a high level
(`The GNU ChangeLog
<https://www.gnu.org/prep/standards/html_node/Change-Logs.html#Change-Logs>`_
standard is worth a read),

* be word-wrapped to 72 characters per line, don't go over 80; and

* separated by a blank line from the first line.

* Bullet points and numbered lists are okay, too::

Expand Down Expand Up @@ -171,6 +282,8 @@ This page was composed from material found at

* http://www.catb.org/esr/dvcs-migration-guide.html

* https://git.dthompson.us/presentations.git/tree/HEAD:/happy-patching

* and other places.


Expand Down
36 changes: 0 additions & 36 deletions doc/development/quickstart.txt
Expand Up @@ -39,43 +39,7 @@ Quick-start
.. _PyInstaller/init.py: https://github.com/pyinstaller/pyinstaller/blob/develop/PyInstaller/__init__.py


.. _Guidelines for Commits:

Guidelines for Commits
=============================

* **Commit often and in logical chunks.**
A commit should be one (and just one) logical unit. It should be something
that someone might want to patch or revert in its entirety, and never
piecewise. If it could be useful in pieces, make separate commits.

* **Write meaningful commit messages.**
Using atomic commits will result in short, clear, and concise commit messages.
Non-atomic commits make for awful run-on commit messages.

* Try to make small patches (i.e. work in consistent increments).

* Separate changes that affect functionality from those that just affect code
layout, indendation, whitespace, filenames etc. This means that when looking
at patches later, we don't have to wade through loads of non-functional
changes to get to the important parts of the patch.

* Especially don't mix different types of change, and put a standard prefix
for each type of change to identify it in your commit message.

* Restrict all whitespace changes to a specific type and document as such.

* Restrict refactorings (that should not change functionality) to their own
commit (and document).

* Restrict functionality changes (bug fix or new feature) to their own
changelists (and document).

* If possible, commit often. This helps to avoid conflicts.

* Only push when your tree passes validation: see TestingPatches.

* Discuss anything you think might be controversial before pushing it.

.. include:: ../_common_definitions.txt

Expand Down

0 comments on commit 1f092e1

Please sign in to comment.