topbanner_forum
  *

avatar image

Welcome, Guest. Please login or register.
Did you miss your activation email?

Login with username, password and session length
  • Friday March 29, 2024, 10:11 am
  • Proudly celebrating 15+ years online.
  • Donate now to become a lifetime supporting member of the site and get a non-expiring license key for all of our programs.
  • donate

Poll

Do you diff / review your code changes before you commit them to source control?

Yes, every single change.
6 (85.7%)
Yes, but only some of it.
1 (14.3%)
No, I don't think it's necessary.
0 (0%)
No, I think it's a waste of time.
0 (0%)

Total Members Voted: 7

Author Topic: Diff before commit  (Read 4500 times)

phitsc

  • Honorary Member
  • Joined in 2008
  • **
  • Posts: 1,198
    • View Profile
    • Donate to Member
Diff before commit
« on: November 04, 2010, 07:45 AM »
I was discussing this question with a colleague recently and we were of quite opposite opinions. I'm a proponent of diffing before commit, I consider it to be kind of my personal mini code review. He basically considers it to be a waste of time, saying that he knows what he changed and that he's just committing very often (which I try to do as well, by the way, but never mind).

I'd like to know how you people feel about this question.

Shades

  • Member
  • Joined in 2006
  • **
  • Posts: 2,922
    • View Profile
    • Donate to Member
Re: Diff before commit
« Reply #1 on: November 04, 2010, 08:56 AM »
For me 'Diff before commit' is THE golden rule, with the 'Commit often' rule a very close second.

It is definitely not a waste of time, because you will lose a lot more time when reviewing bigger pieces from other developers and how their changes will affect yours.

You can use 'lazy methods' for coding...never for committing!

f0dder

  • Charter Honorary Member
  • Joined in 2005
  • ***
  • Posts: 9,153
  • [Well, THAT escalated quickly!]
    • View Profile
    • f0dder's place
    • Read more about this member.
    • Donate to Member
Re: Diff before commit
« Reply #2 on: November 04, 2010, 09:54 AM »
Depends on what I've been doing.

If I've been working on something for hours before committing, I definitely do look at diffs in order to write a proper commit message. For shorter-interval commits, the changes are usually small enough that I can remember all of what I've been working on. The more often I commit, the less need I have for looking at diffs. Also helps a lot to work on a single feature at a time (I need to force myself in the habit of doing that - take note if I find other stuff that needs bugfixing/refactoring and modify that after a feature is complete, or do it in a second branch).

A few times, the pre-commit diff has caught silly bugs... but that has always been tied to not committing often enough.
- carpe noctem

Eóin

  • Charter Member
  • Joined in 2006
  • ***
  • Posts: 1,401
    • View Profile
    • Donate to Member
Re: Diff before commit
« Reply #3 on: November 04, 2010, 10:42 AM »
When doing a commit through TortoiseGit, it lists the changed files and double clicking on one shows a diff. So I usually check the diffs only if I spot something out of place in the file list.

housetier

  • Charter Honorary Member
  • Joined in 2005
  • ***
  • default avatar
  • Posts: 1,321
    • View Profile
    • Donate to Member
Re: Diff before commit
« Reply #4 on: November 04, 2010, 11:34 AM »
I always diff every single file before commiting. I don't make exceptions, I have caught many mistakes this way.

Since I also commit often, the changesets are small so it is easily and quickly done.

I find it difficult to believe for someone to know of all the changes that were made. I know that not double-checking will lead to mistakes: our company hired subcontractors and we regularly had to remove their debug statements and such from the code (only if we saw them).

In our team we do several things:
  • branch early
  • branch often
  • commit early
  • commit often
  • push early
  • push often

Of course before you can push you have to pull, but our git server will remind us :)

I was praised for the quality my work and got a raise recently. I will continue to double check every change, every file. "diff" can save you a lot of trouble!

mwb1100

  • Supporting Member
  • Joined in 2006
  • **
  • Posts: 1,645
    • View Profile
    • Donate to Member
Re: Diff before commit
« Reply #5 on: November 04, 2010, 11:44 AM »
For me, diff-before-commit is in the same category as test every change before commit.  When I decide not to do it - usually because the change was so small, that there's *no way* it could cause a problem (like fixing a spelling mistake in a comment) - invariably some dumb-ass thing breaks the build or causes a crasher bug.

And diff-before-commit is really a pretty quick and easy sanity check (it's not like I review every changed line - it's more of a quick verification that I'm checking in what I'm expecting to check in, and not checking in stuff I shouldn't be).


sagji

  • Participant
  • Joined in 2010
  • *
  • default avatar
  • Posts: 25
    • View Profile
    • Donate to Member
Re: Diff before commit
« Reply #6 on: November 08, 2010, 10:20 AM »
For me the diff before commit is the final approval step.
It is were I make sure of code quality - conformance to coding style and naming convention.
Make sure that any test code is removed.
Make sure I have tested all code paths.
Make sure that all changes correspond to something in the change description - even if it is just "review changes"
Make sure that all required changes have been done.

The only time I don't do the diff is if there is no decision in the change - such as adding in an update from an external source (e.g. updating to a later version of a 3rd party library.)