DEPRECATED

This space is obsolete and unmaintained. The entire content of the DM Developer Guide is now maintained at https://developer.lsst.io .

You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 8 Next »

Organizing changes into commits ( DM-1181 - Getting issue details... STATUS )

Follow the practice for structural organization of commits described in the OpenStack Best Practices document. From the linked document:

The cardinal rule for creating good commits is to ensure there is only one "logical change" per commit. There are many reasons why this is an important rule:

  • The smaller the amount of code being changed, the quicker & easier it is to review & identify potential flaws.
  • If a change is found to be flawed later, it may be necessary to revert the broken commit. This is much easier to do if there are not other unrelated code changes entangled with the original commit.
  • When troubleshooting problems using GIT's bisect capability, small well defined changes will aid in isolating exactly where the code problem was introduced.
  • When browsing history using GIT annotate/blame, small well defined changes also aid in isolating exactly where & why a piece of code came from.

Things to avoid when creating commits:

  • Mixing whitespace changes with functional code changes
  • Mixing two unrelated functional changes
  • Sending large new features in a single giant commit


Rule of Thumb

If a code change can be split into a sequence of patches/commits, then it should be split. Less is not more. More is more.

Authors and Reviewers

Authors: rebase as necessary to satisfy the above.
Reviewers: check that commits are logically broken up when reviewing the code; request the author to rework them if they're not.

 

Contents and format of commit messages ( DM-1182 - Getting issue details... STATUS )

Follow the practice described in a blog post by Tim Pope (or the very similar Perl commit guidelines, excerpted in the first comment to DM-1182).

Example from the blog post:

Capitalized, short (50 chars or less) summary

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 imperative: "Fix bug" and not "Fixed bug"
or "Fixes 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, followed by a
  single space, with blank lines in between, but conventions vary here

- Use a hanging indent

 

Authors and Reviewers

Authors: Not every commit requires a long message; use your judgement as to which ones do and which ones don't.
Reviewers: Verify commit messages are reasonable when reviewing the code. Request fixes if they are not.

 

  • No labels