On Fri, 2013-10-11 at 11:09 +0300, Nikolai Kondrashov wrote:
On 10/10/2013 11:27 PM, Simo Sorce wrote:
> On Thu, 2013-10-10 at 15:46 -0400, Stephen Gallagher wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On 10/10/2013 10:07 AM, Nikolai Kondrashov wrote:
>>> Make DEBUG macro accept variable number of arguments, thus removing
>>> the need to wrap the format string and its arguments into parens
>>> on invocation.
>>
>> Nack.
>>
>> Please do us a favor and split this into two patches, one that makes
>> the change to the macro definition and one that bulk-updates the
>> existing messages. Having a 2.2MB patch file with a few small changes
>> buried in it somewhere is very difficult to review. I'm not confident
>> I can locate the real changes amidst the find-replace noise.
>
> I would not want to consider such a patch in any case. The churn is just
> too extraordinarily big and I do not see enough value to warrant it.
>
> The patch is so immense that is it impossible to review, so we would
> have to make a leap of faith in assuming every single change did not
> introduce errors.
I'll try to reduce the amount of faith required. Maybe try a semantic patch
[1], or do some metrics on the code and the changes.
> I am not against 'simplifying' the debug macro if possible, but then it
> should probably be done in a backwards compatible way and debug
> statements changed in time, just like we did when we switched from mere
> numbers to error type definitions.
>
> For example we could call the new macro DBG, an start using it in place
> of DEBUG in new code. DEBUG would be just a wrapper around DBG that
> maintains the old interface.
I would advise against that. This will hurt maintainability, and I think
having two ways to specify debug level is harmful as well. This is how
software rot happens. It is better to have a way to verify that such changes
don't break anything (tests?) and do a change globally, or not change anything
at all.
However, this is of course entirely up to you and I won't push this further as
I'm only doing it for a sport, while waiting for Beaker job queue to unclog.
Sincerely,
Nick
[1]
http://coccinelle.lip6.fr/sp.php
I seriously like the simplification, but I am dead against such a
monstrous patch that touches *all* the code.
Although not ideal I am perfectly fine with changing code gradually like
we did (and are still doing) with the debug numbers.
Simo.
--
Simo Sorce * Red Hat, Inc * New York