On 10/11/2013 10:09 AM, 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 like this idea. Actually this might be good opportunity to rename our
DEBUG macro and put it to proper namespace. The new macro could be
called SSS_DBG. Then we can rewrite the old code module by module (this
would be long term task) and use the new macro in new code. Later when
the DEBUG is not used anywhere we can remove it.
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
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
