last week in a codereview i got asked to clean up indentation of some html structures that i had added.
my response was: i duplicated an existing structure and made mine look exactly the same.
i agree that proper indentation would be nice. but if i did that, my structures would look different from the original. i could clean up the original too, but then the diff would be harder to read because it would contain unrelated changes.
copying the original unclean structure in this case is best for readability, because consistency is more important than any particular style.
if anyone cares about the style, they can submit a style cleanup separately.
> my response was: i duplicated an existing structure and made mine look exactly the same. i agree that proper indentation would be nice. but if i did that, my structures would look different from the original. i could clean up the original too, but then the diff would be harder to read because it would contain unrelated changes.
The idealist part of me wants to say: Clean up the original code, submit that as the first diff, then rebase your additions on top as a separate diff.
The pragmatist part of me recognized long ago that a lot of software engineering now is like what you describe. The days of elegant, carefully-maintained codebases where someone sweated over every semicolon are long gone.
in a pull request all commits get merged into one when you review the PR, so it would have to be two separate pull requests to be reviews separately, which is what i suggested.
when working by myself without PRs then i would do almost exactly as you describe. i would probably put the cleanup after the code change though mainly because i want to focus on solving the problem first, and worry about cleanup later. but i can see the benefit of doing the cleanup first.
My feelings about that stuff have shifted over time - these days I figure fuck it let the linger worry about the formatting. As long as everyone on the team is using the same config, things like indentation prettt much take care of themselves.
A guy I know at $currentjob would have agreed to the criticism, then created at least 2x Jira tickets to clean up the original and then the structure you mention.
last week in a codereview i got asked to clean up indentation of some html structures that i had added.
my response was: i duplicated an existing structure and made mine look exactly the same. i agree that proper indentation would be nice. but if i did that, my structures would look different from the original. i could clean up the original too, but then the diff would be harder to read because it would contain unrelated changes.
copying the original unclean structure in this case is best for readability, because consistency is more important than any particular style.
if anyone cares about the style, they can submit a style cleanup separately.