User: Password:
|
|
Subscribe / Log in / New account

Creating a Proper Path for Clean-up Patches

Creating a Proper Path for Clean-up Patches

Posted May 29, 2008 8:38 UTC (Thu) by ekj (guest, #1524)
In reply to: Creating a Proper Path for Clean-up Patches by midg3t
Parent article: Getting the right kind of contributions

Perhaps.

But there's also the point that sometimes two patches:

1) This patch does NOTHING other than reformat function foo()
2) This patch adds a check for $ERROR_CONDITION to foo()

Is easier to read and understand the patches than it would've been if they where rolled into
one.



(Log in to post comments)

Don't do it then

Posted May 29, 2008 9:24 UTC (Thu) by khim (subscriber, #9252) [Link]

If reformatting of the patch is such a big part of it that it's hard to understand what the patch is actually doing - then drop the reformatting part. It's as simple as that. Formatting patches are bad. We once hired some guy and asked him to add small functionality to the program. It was supposed to be week or two of work. He took month and at the end we've got requested functionality and... patch which changes 90% of code to make it adhere to coding standards. It was the last month this guy worked for us: the work was redode by someone else.

The rule of thumb is simple: you must be ready to answer for all lines your patch touches. You are "owner" of said lines after patch is applied (most VCS will allow you to find such owners). And answer like "I've only moved this two whitespaces right" does not count: you should genuinely understand this code. If you can do it - feel free to reformat code. If you don't have interest or time - leave the code as is!

Don't do it then

Posted May 29, 2008 11:09 UTC (Thu) by ekj (guest, #1524) [Link]

It's not that it's hard. It's just that -any- noise subtracts from readability.

Which means a patch that -only- does the minimum required for adding the error-check will
always be more readable than one that does that, plus fix the formatting of the surrounding 5
lines.

So your advice adds up to, essentially: NEVER FIX STYLE-ISSUES.

Which is overdoing it. I agree that it's *often* a bad thing to do style-change. But there's
two important readability-issues here:

One is the readability of the patch. That is always better with zero style-changes.

The second is the readability of the code after the patch is applied. If that is sufficiently
improved by doing style-fixes, those may be worthwhile even though it hurts the patch.



Copyright © 2017, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds