Comments on Andrzej on Software: Small, deployable commits

2011-12-30T19:39:58.891+01:00

This comment has been removed by the author.

Thanks for all comments! First, a clarification -...

2011-12-30T19:39:58.670+01:00

Thanks for all comments!

First, a clarification - when I talked about a commit, I meant a git-like commit, however it's not really that different in this context from Subversion-like comment.

@poulwiel:

I like the idea of having a quick feedback. That's why i love TDD, that's also why I like continuous deployment.

@szeryf

When you have a single commit broken (in git) it doesn't break your friend build.

Good point with metric_fu.

@jasiek

I hate feature branches, too complicated for my small brain :)

You're right about the possibility of reverting single commits.

@Tartley

You, sir, must come from a very good school of software development :) I agree with everything you wrote. I'm sure your team-mates will gain a lot when you show them how to refactor with small commits, being green all the time. That's how I learnt it. Pairing with someone who can do that makes miracles.

@ncr

Reviewing code before a commit may work fine in case of pair programming. In other cases the feedback is too slow and I hope you're not blocking anybody from commiting their changes. I can't see the advantages of such processes.
Criticisism is fine :) We sometime cross the line and go into haters-mode, but that's fine. I hate ugly code :)

We review code before each commit (even css-relate...

2011-12-30T19:30:34.989+01:00

We review code before each commit (even css-related stuff) and we like it, taking a constructive criticism easily has never been a problem to anyone in our team.

Keep writing :)

very interesting points of view, i don't know ...

2011-12-30T16:03:46.324+01:00

very interesting points of view, i don't know how about You all but i fell that we all think about something else while saying "commit", at least what this "commit" will lead to (i.e. if team is working on one repository or distributed repos), as a consequence we are writing about something slightly different - still it's very interesting read :-)

Should I refactor before every commit? Definitely ...

2011-12-30T14:45:38.957+01:00

Should I refactor before every commit? Definitely not, IMHO. :-)

I prefer that refactoring is separated out into its own commit. Then others can clearly see which changes are behaviour-preserving, as opposed to actually adding or modifying functionality.

I think the urge to keep work private because it isn't finished (e.g. isn't refactored) should be resisted. It's better to be public and transparent about what you're up to. The sooner others see your changes, and the sooner CI gets to run against them, the better for everyone.

Should a commit pass all tests?
I've settled on a style where every commit is deployable, and passes all tests. Sometimes this means labelling the in-progress functional test as 'expected failure'. (Python unittest has a decorator for this, that ignores the failure in test results.) Martin Fowler's book 'Refactoring' describes how to do this in the trickier situations, such as when making a sweeping change that affect lots of code.

However, some of my current team-mates don't believe this is possible all the time, and for those times they continue to use feature branches to keep the non-deployable code isolated until it can be made working, then merged with the main development branch. I plan to pair with them on the next such change, to discover which of us is right.

As often as possible - my main reasoning being tha...

2011-12-30T13:09:58.974+01:00

As often as possible - my main reasoning being that having a finer granularity of changes lets me revert a commit much easier in case of something going awry.

We've experimented with pre-commit code reviews with SVN a while ago, and that has worked okay. Our process at the moment uses reviewboard with a branch-per-feature approach - eg. a feature is started, reviewed and finished within a branch, and only then merged into a branch that is later deployed.

Should a commit pass all tests? Definitely. Otherw...

2011-12-30T11:47:00.051+01:00

Should a commit pass all tests? Definitely. Otherwise, it will mess your colleague's work when they do the update -- they won't know if the the test failed because of what they did or what you did.

Should I refactor before every commit? Definitely (at least IMHO). Code that barely passes the tests is just a half-product. A sort of prototype. Something that works, but you wouldn't show it to your colleagues, because it's ugly, repetitive etc. I wouldn't want somebody else to see it or comment on it, while I'm still working on it. For me it's just unfinished.

Also, when you have something like metric_fu installed on your continuous integreation, non-refactored commit will likely fail the build. And you get to wear the stupid hat :)

not every commit has to be deployable, i'm wor...

2011-12-30T10:30:36.815+01:00

not every commit has to be deployable, i'm working on my repo so pulling from it (to the main repo) will be done when things will be ready. it's just my opinion. i find comfortable that i can see the changes from master repo perspective to someone elses repo and decide if it is ready or not.