this is not at all related to the essence of the commit, but as the rails guys are using git which brings us the beauty that is rebase -i, why is this commit so incredibly bad?
It mixes functional changes and whitespace changes. It even contains functional changes not related to the commit message.
I would never dare to submit a commit like this into our internal repository, much less to a public one.
I know. Just nitpicking, but having the possibility of creating "clean" commits in a reasonable amount of time is one of the nice features of git. Why not use them?
"I would never dare to submit a commit like this into our internal repository, much less to a public one."
Never? Are your deadlines infinitely flexible? I try my best to adhere to single purpose commits too, but being under the gun I know I've slipped on a few (hundred) occasions. And no, I don't go back and split commits.
was I still using SVN, I would totally agree with you here, but git was designed to help with issues as the ones that are in the patch:
Git diff is awfully useful in displaying things like inconsistent newlines, inconsistent whitespace and it helps you keeping whitespace and code changes separate.
And then there's "git add -i" which makes it very easy to go through a file change by change and commit parts of the changes. This feels dangerous, but to keep whitespace changes separate from functional changes, it's very powerful and not dangerous at all.
Fixing a commit like this takes maybe 20 seconds more than just committing it and considering the amount of time you save when you try to fix the bug in that commit (of course there is one), it's totally worth the hassle.
Note that this relies on git features. If you don't have git, I totally agree with your point. But my comment was about a commit made with git.
To make a point: Have a look at the last change in the commit. How are these <th> tags related to the HTML5 conversion?
Now when they go back to the changes in a few weeks, wouldn't they be glad to see these changes in its own commit with its own message?
After all, that will make it all the more fun the next time you are under a deadline, and you are wondering why that commit broke something. You have the added benefit of having to imagine what you were thinking a few months ago, in addition to debugging. Fun!
In other news, writing tests is a waste of time when you are under a deadline. It mostly works in the browser!
Ease up on the sarcasm. I get it, you write perfectly descriptive commit messages and your apps are perfectly tested. I was cutting the original committer some slack because he was guilty of something that most of us are guilty of.
It mixes functional changes and whitespace changes. It even contains functional changes not related to the commit message.
I would never dare to submit a commit like this into our internal repository, much less to a public one.
I know. Just nitpicking, but having the possibility of creating "clean" commits in a reasonable amount of time is one of the nice features of git. Why not use them?