Seems obvious, right? You just edit it until it works the way you want it to. Not so fast. Reckless code changes will turn your project into a big ball of mud before you know it. Done in the right way you'll experience less frustration and fewer crises.
While you may write code as a developer, your main task is not writing code: it's removing ambiguity and mitigating project risks.
If you're working on a personal project or in a small startup this is not for you. This is written for the developers working in medium to large teams on mature codebases.
Small commits, named well
Keep your commits small and related to a single piece of functionality. This allows them to be easily reverted later if necessary.
Give your commits a descriptive summary - one which will help another developer to find the commit which introduced a particular feature (or bug!).
Bad:
"TAG-789"
"added some more code"
"addressed merge request comments"
Good:
"Split duplicated image analysis code out into its own function"
"Added GET param to QR code image URL to avoid cache"
Small merge requests
Where possible merge requests should be kept small. The more lines of code touched in the MR, the more likely it is:
- to make your colleagues reluctant to review your code (especially since they're often under no obligation to do so)
- for you to introduce bugs into the codebase (there's a direct relationship between line count and bug count)
- for your reviewers to get fatigued and miss things
Sometimes you might be updating a library and you might have to create a huge merge request. That's ok - it's an exception. The rule is, keep your MRs small.
Incremental improvements
Suppose the team has identified the use of some particular pattern is, in fact, an anti-pattern. Unless you've been given the greenlight by your tech lead or dev team manager, do not go and strip out every occurrence of that pattern! Doing so introduces risk to the project. Instead, first raise a ticket to remove the offending pattern from the codebase. Then during the course of normal (unrelated) development:
- if you're touching an offending pattern, re-factor it
- if you're touching a function containing an offending pattern, re-factor it
- if you're touching a file containing an offending pattern but it's unrelated to the code at hand, add a
# TODO: refactor in [ticket number]comment
With any luck there'll be few to no occurrences left by the time the ticket is made priority.
Never write code without a ticket
Writing code is easy. Writing good tickets is hard. However that ticket is there to protect you.
- Your colleague has a brilliant one-liner which makes the feature you're working on even better? Tell them to raise a ticket.
- The tech lead asks you what it'll take to implement X or Y feature? You reply with
2 * (your best estimate). Perhaps offer to raise a ticket for them if one doesn't exist.
Including code which falls outside the scope of your ticket is scope screep. Every new line of code increases the probability of new bugs and therefore project risk. Additional code requires additional testing which delays the completion of the feature. Delays add pressure to everyone in the team.
So play it safe: ensure the code you're working on is part of a ticket and within scope.