When you are repairing other people's code, don't change it to your style, fix it in theirs.
When you are repairing other people's code, don't change it to your style, fix it in theirs.
As we've moved, here, from SVN style version control systems towards git we've encountered a few snags along the way. People used to SVN want to check out files, make a lot of changes, complete the whole change request in a single checkout (which could take a while), and then check it back in. The problem arises that they end up making tweaks (small refactors, style adjustments) that aren't really part of the change request. This creates confusion when trying to check the work back in and review it. It also means that positive changes (those tweaks usually are) get tossed out when the whole CR is rejected for some other reason.
Git makes it much easier to do small commits. Changed a file to use spaces instead of tabs (per style guide), check it in with just that change. Refactored a common set of lines into a new function? Check in the new function creation once. And then do one check in per true refactor (replacing the many lines with one function call).
This allows you to cherry pick much more easily, but also to isolate errors. Maybe that Extract Method refactor should only have been done to 10 of the 12 cases you did it in. Or what looked like a common magic number was only coincidentally the same. If you made all the changes in one commit, it's harder to revert just the broken part. And git bisect is useless if you have a handful of giant commits, rather than many smaller commits.
If you'll be the one owning the code, do what you want. If it's not yours, check first.
If you do have communal ownership, coordinate and establish style guides, ideally using tools to establish conformance (rather than eye-balling it, this also leads to greater consistency).
This randomly reminded me of the time I let my friend use my laptop, only to find out the next morning that he had changed all the colors and styles of my windows xp with a gimmicky "mac skin". So infuriating.
This way if someone decides to rearrange something, the stakeholders can hold up the review until they're happy.
I've certainly blocked code reviews because of planned changes.
That being said, you only "own" code if you own the business. If you think you "own" code, you might need to re-evaluate your ego.
Suddenly an SVP pulls you aside and tells you you need to extend a service that team SadStyle owns and integrate it with your own team's widgets. Team SadStyle has regrettably not yet been shown the way by a C# bodhisattva and has the following inarguably egregious C# style errors in its codebase:
1. Braces open on a newline instead of on the same line
2. Profligate use of `var` for every kind of variable declaration
3. Inconsistent use of tabs and spaces
4. Anonymous functions sprinkled into the code ad hoc that are sometimes quite long
You're shocked at team SadStyle's code, but you need to get some work done, quick, since a deadline is coming. So what are you going to do?
If you answered "I'm going to go through and change every brace, variable declaration, and span of whitespace", I think you should maybe think more about whether this is a good use of your time.
Personally I try to follow the style of the existing code, but when something permamently becomes the responsibility of my team I won't hesitate to apply the style of my team to the whole thing. Most parts of the style can be applied automatically, and the parts that can't be will be applied when functional changes to that part of the code are required.