We've had an interesting discussion at our stand-up today. The discussion was triggered by Jan Filipowski post on "early commiting in TDD".
The main questions are:
- Should I refactor before every commit?
- Should a commit pass all tests?
- What units of code should be code-reviewed?
It got me thinking about my current approach and about my "ideal world" approach.
I'm a big fan of the idea of continuous deployment. It means that every commit, after passing all tests, can be automatically deployed.
I'm not there yet. I try to work with very short commits, often it's only 2-line changes.
However, I still miss an infrastructure which could automatically push
this commit, test it and deploy. It's not that it's difficult to
prepare, it's just some kind of a psychological bareer. It's definitely
something I want to improve as soon as possible.
Sometimes it's hard to have deployable commits. Recently we've been
upgrading our app to Rails 3.1 and it's not something that can be done
in one commit.
Going back to questions:
Should I refactor before every commit?
Refactoring is part of a workflow, but I wouldn't say it should part of
every commit. Sometimes a commit can be just the red-green part, and
it's the next commit that does the "refactor" part.
Should a commit pass all tests?
This is a tricky one. In my ideal world - yes, because it should be
deployable. Is it hard to achieve in practice? I'm wondering if there's a
conflict between a deployable commit and one that fails on some tests.
Maybe it's fine to have some tests failing, if they are not part of the
code that is being used in production?
What units of code should be code-reviewed?
We tend to make code reviews using GitHub commit comments. They're fine,
but sometimes I comment on something that was changed 3 commits later,
which makes my comment irrelevant.
Obviously it all depends on a project. In most cases I think we
shouldn't be too serious with the code reviews comments. In my opinion
code reviews should help us triggering some coding/architecture
discussions, they shouldn't be too formal. I don't like proceses which require code reviews before the code goes into production
- we should trust each other that nothing stupid gets deployed. If we
treat a code review as a tool for better communication then it's not
that important if a comment is about a code that changed later or not.
How about you? How often do you commit?