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?