Rules for working together, staying sane, and shipping quality product.
Open source project? Team project? Product? Client work? Whichever you’re working, have DVCS will travel.
There’s nothing ground-breaking about these commadnments, but they’re good to reiterate.
The point of commit messages is to explain in brief to other developers, including your future self, what changed and why. This makes going through a commit log much easier, it makes it easier to understand a merge request.
Good commit messages are like good email messages. The subject summarizes the story (the what) and the message explains the details (including the why). And like a pithy email message, the body isn’t always necessary if the change can be accurately summed up in the title. And formatting matters. The explanation from Subsurface is a good start.
Commit messages notwithstanding, the diffs in the commits should tell a story of what changed. They might not explain the intent, but like a tight plot they should include the necessary elements to the exclusion of distractions.
This means not mixing up other unrelated changes or refactoring including reformatting. That one line change that includes 40 lines of whitespace changes? Two commits are better if only to break apart the intent of the commits.
The importance of commit ordering mostly has to do with merging branches. It’s very convenient to simply merge in a new branch or merge upstream changes into a working branch. The result though is typically a mess of merge commits and a “zipper” like merge of commits based on their chronology.
What you want to know, however, is where and when something in the codebase changed - and the answer is not a timestamp, it’s between a sequence of applied changes. So these changes should be recorded in an order meaningful to application.
Rebase, do you use it? Rid thyself of merge commits. You’ll lose a truly chronological ordering of your commits, but this truly matters very little compared to having an accurate record of when changes where applied.
For features and fixes, but perhaps fixes especially, changes should be atomic. Like, really atomic. More often than not the intent is to address one feature, one issue, and the code should do just that. No additional tweaks or features that someone thought of along the way. This makes testing, evaluating, and merging much more straightforward.
A merge or pull request with extraneous changes requires extraneous review and may require throwing the baby out with the bathwater.
So start from an agreed upon authoritative branch. Master or production if that’s how you roll, or development if it’s git-flow, or what have you. But start branches from that point. And make only the changes needed for the fix or the feature or even the format refactoring, if that’s what it is.
Just keep it atomic!
Those atomic changes, how do they work? Do they fix a bug? Was a test added for the bug to verify the change actually fixes it?
We all know how easy it can be to just write the code and ship it. But the overall speed of the team increases when everyone has assurance that features and fixes added to the codebase actually work.
The very least you can do before even pushing a changset is running the existing tests.
Do the changes break something else? Bang for your buck, just running the existing tests beats even adding new tests. It’s easy to scoff at the value of silly tests like checking that a Django view loads correctly, but it’s a good sanity check later to ensure that the little tiny change didn’t muck up something else.
At the very least run the existing tests to ensure new code doesn’t break something that was working just fine.
This stuff does not go into source code. Ever.
Look, we’ve all done it at one point or another, due to accident or naivete. You do it once (maybe twice) and then don’t do it again. Right?
What shouldn’t go in? Passwords. API keys. File paths. These things should all be provided by the environment or at the least by some configuration file outside of source control. This isn’t just about security. It’s about being able to move the code around, to deploy to different environments (staging vs production), and to make setting up for development straightforward.
This one is about context sanity.
Every language has a coding style in addition to its syntax and this should be respected. Not because that style is good but because doing otherwise adds surprises. Morever, barring a wholesale change, a project’s existing style should be respected. Yes, even when it’s wrong.
The goal here is a level of consistency so that the development team knows what to expect. This goes for comments, too. You’re never going to get pure consistency and that isn’t the goal. Everyone has their own quirks that will come through. But a common style reduces the cognitive load just a little bit.