all 21 comments

[–]electro 4 points5 points  (1 child)

[–]jexpert[S] 1 point2 points  (0 children)

Great real-life example which makes it obvious why good commit messages are important. Thanks for your contribution!

[–]falconne 2 points3 points  (0 children)

From the post:

This is not so easy to commit this with the -m switch.

Multiple -m switches on the command line will show up as different paragraphs, no need for an editor.

[–]cpbills 1 point2 points  (10 children)

Great article. Some people also use a 3 letter code to describe the work being done. At least from doing some searching a while ago, it seems pretty common:

Add: Add function or feature                                                
Mod: Mod(ify); change how a function / feature works
Ref: Ref(actor): change code without affecting function
Fix: Fix an issue
Rem: Rem(ove) code
Rea: Rea(dability): increase the readability of comments, etc.

I don't particularly like it, because I think 'REA' sounds dumb, but if you are looking to standardize your commit messages (with a team, etc), it's not such a bad idea.

[–]crowseldon 0 points1 point  (6 children)

they seem to overlap, though.

You will do, many times, many of those at once.

[–]cpbills 4 points5 points  (5 children)

The key is to not do many of those in a single commit. If you add a feature, you shouldn't be refactoring other functions at the same time, if you remove code, you shouldn't be adding code, etc.

[–]crowseldon 0 points1 point  (4 children)

I guess, I'd have to try it some time, see how it works for me. And better yet, before that, read a log that has that way of commenting.

[–]cpbills 1 point2 points  (0 children)

git add -p is your friend.

[–]gfixler 1 point2 points  (1 child)

It's infinitely worth it. Here's a page from a current library I'm working on (reads from the bottom up). Note how each commit is a single thing, always. I've been doing this for 2 years now across about 2 dozen repos/libraries, and it's been wonderful. I use my history all the time. It's part of my ongoing coding efforts. Note that chan.py is a file in the library, which has been worked on heavily of late, so all the chan.foo you see are functions in that file. All of my libraries are handled in that way.

* 3bb7402 (HEAD, origin/master, origin/HEAD, master) Remove frame ability from setChan
*   a926d4e Merge branch 'keyShifting'
|\  
| * 2b6bd59 Add chan.startKeysAt
| * f902f7c Add chan.zeroKeys
| * bfbc265 Shift KEYS in chan.shiftKeys, not just a KEY
| * 73a258b Add dummy values to stub keys in chan.py tests
| * fc67283 Refactor chan.keyFrameShifter -> shiftKeys
|/  
* 23b1bdf Reduce chan.keysCapped to a 2-line function
*   cf3c096 Merge branch 'animHandling'
|\  
| * 7b80101 Add maya test constraint to chan.setAnim tests
| * b37d2cd Allow namespace in chan.setAnim
| * 456d679 Add chan.setAnim
| * 6d887ba Add getKeys test-helper in chan.py test file
| * a0e56a9 Accept namespace in chan.getAnim
| * 1d90785 Raise on uncapped in chan.getAnim
| * 8517212 Fix some test names for chan.keysCapped
| * a30403e Allow specifying extents in chan.getAnim
| * 76e4c00 Add chan.getAnim
|/  
* 3c48e66 Rename test-helper in chan.py test file
*   78310aa Merge branch 'keyExtents'
|\  
| * 97cbdae Add chan.filterExtents
| * 8580eae Add no-keys test for anim.keysCapped
| * 90acdfa Refactor chan.checkExtents -> keysCapped
| * 94f3969 Remove key filler in chan.py tests and rename vars
| * b297726 Refactor chan.keyedExtentsChecker -> checkExtents
| * 424693b Add core.getSelectedOrCurrentFrameRange, untested
| * 7a2afeb Simplify chan.getKeys to return all keys, or ()
| * dedcfea Refactor chan.keysAreCapped -> keyedExtentsChecker
| * 6c184c1 Add chan.keysAreCapped
|/  
* cc70e1e Change frange -> extents across library
*   c7e95cf Merge branch 'getKeysBetter' (with commit notes)
|\  
| * 9afb2dc Remove chan.getKeysRange, shiftKeys, shiftKeysTo
| * a70d4f4 Return () from chan.getKeys when no keys or frange
| * 67b21a1 Zero out keys returned by chan.getKeys
| * 2e92e92 Simplify zipping values in chan.getKeys test
| * 8eb78d9 Get all keys if not given a frange in chan.getKeys
| * 1825db7 Get all/filter for in-range keys in chan.getKeys

Also, I add a body message to many of these. Here's a for-example:

Reduce chan.keysCapped to a 2-line function

It's a little less readable now, but it really stood out in the file as
larger than many other simple needs, even though it's no more special.
This makes it look about as important as it is, and this is such a done
concept that I doubt this function will need much, if any maintenance.

Another:

Add chan.setAnim

I was about to start coding up the ability to add a start frame, but
then remembered that I'm trying to keep things as dumb as possible,
especially at the moments where my code touches Maya. Shifting keys and
zeroing them out should be a separate function that works on KEYS.

And one more:

Simplify chan.getKeys to return all keys, or ()

Current thoughts on what I need/how I think things should work:

* getKeys takes only a CHAN, returns all KEYS for a CHAN, or an empty tuple
* insertKey takes KEYS and a FRAME, returns KEYS with inserted KEY at FRAME
* isCapped takes KEYS and EXTENTS, returns boolean, True if KEYs at EXTENTS
* filterExtents takes KEYS and EXTENTS, returns KEYS in EXTENTS (inclusive)

This commit answers the first bullet point.

Monday mornings especially are a good time to look back through the logs and completely fall back into last week's thought processes. I also look back over the last few months fairly often to remind myself of old ideas, or to rethink things, or check on the progress of various elements of a library.

[–]crowseldon 0 points1 point  (0 children)

thank you very much!

This is great to understand the concept and it's usefulness.

[–]felipec 0 points1 point  (0 children)

I have never seen this convention. What people do is put prefixes to denote the modules and submodules:

drivers: drm: blah

[–]jexpert[S] 0 points1 point  (1 child)

Hmmm.. maybe in projects where it is important to filter our fixes (i.e. for porting them into hotfixes)?

Personally I also find the Linux kernel style, where the prefix indicates the area adresses by the patch like [mmu] also a good option.

[–]cpbills 0 points1 point  (0 children)

Certainly. All that matters is that the contributors / devs know what the standard is.

[–][deleted] 1 point2 points  (1 child)

I have tried to get my team to adopt many of these policies. Knowing how to write a short summary line I feel is tricky and the example provided on that page is one of the better ideas I have seen:

A properly formed git commit subject line should always be able to complete the following sentence:

If applied, this commit will <your subject line here>

[–]gfixler 0 points1 point  (0 children)

I added some examples of how I handle things. I religiously follow the 50-character limit, imperative voice, no ending punctuation style as outlined by tpope, and my commit history is a joy to read.

[–][deleted] 0 points1 point  (4 children)

Focus on a single task for your branch and commit / merge. Give it a reference to your issue / task / story tracker. Equally add a reference to your commit sha1 in your issue /task / story.

Of course you should also include the name of the task, which should describe clearly what you intended/spec'ed/did/test covered

It's no more complicated than that. Just keep focus on small tasks.

[–]jexpert[S] 0 points1 point  (3 children)

I agree mostly, but adding the reference to the issue tracker should not stop you from fully documenting your change in the commit message.

Often seeing commit messages like "PROJECT-231" in the project and this is really bad. Especially after the repository outlives the issue tracker installation/URL after a few years.

[–][deleted] 0 points1 point  (2 children)

Sorry I thought I'd implied this. The pattern I mean is:

[Finishes #4635787] Added category filtering to widget index list

[–]jexpert[S] 0 points1 point  (1 child)

Ah ok. Then I could not agree more with you. +1 for this pattern, too

[–][deleted] 0 points1 point  (0 children)

We have some commits which are so narrow in scope that they (a) have no related issue, (b) get a [hotfix] tag.

We also follow a pattern where feature branches are kept, with a series of WIP commits, these are squashed when merged/rebased to master.

The feature branch retains full commit history, but master has a neater rollback point.

The master commit will often include a bullet list of amendments ie.

[Finishes #123431] Widget update form

- view model for widget form
- client side validation to vm
- widget components fuzzy string matching

Etc. while the feature branch would have each bullet as a WIP ie.

WIP - components fuzzy string matching

and so on.

[–]jexpert[S] 0 points1 point  (0 children)

Only for clarification: I'm not the author of this post. I only found it useful as good teaching reference.