[SSSD] [PATCH 3/4] Make DEBUG macro variadic
Simo Sorce
simo at redhat.com
Fri Oct 11 12:53:11 UTC 2013
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
More information about the sssd-devel
mailing list