On 06.11.2012 04:47, Kinkie wrote:
On Mon, Nov 5, 2012 at 4:19 PM, Alex Rousskov
<[email protected]> wrote:
On 11/05/2012 02:36 AM, Kinkie wrote:
Hi all,
at lp:~kinkie/squid/sourceformat I have a work branch aiming at
using HERE in debugs() messages wherever sensible.
Apart from the main goal, there's a few typo fixes and a few
wording
improvements. No functional changes.
The patch is ~11k lines so far, with ~1400 changes, I'm about
halfway through.
Any preference on how to review? Or may I just commit once it
passes a
full testsuite build?
These kind of widespread changes will cause a lot of conflicts with
pending patches but will not significantly improve code quality or
functionality in most cases. Is it wise to merge this at all?
There's two benefits IMO:
- in many cases the function names displayed in the messages are just
plain wrong (obsolete) or confusing - there's functions which hop
from
HERE to hand-coded wrong function names.
- it should reduce the memory footprint as HERE strings can get
aliased by the compiler
Note that almost all changes are one-liners.
If the decision is to merge, please merge into trunk _and_ v3.3 (at
the
very least) to minimize porting overheads.
Fine with me. I'll suspend making further changes until a consensus
is reached.
My current preference is to change debugs() to use HERE when the
surrounding code has to be changed anyway OR when there is a good
reason
to add HERE to that specific debugs() line.
I'm aiming at "let's get rid of the pain now" approach, but I don't
want to cause more pain than I'm aiming to solve :)
Like Alex is hinting at if we do it *now* there is more pain than
necessary. This type of change does have "windows of least pain".
Somewhere around Feb next year would be better timing IMO, when we are
about to branch 3.4 for beta. Just like we did massive polish right
before staring 3.3 on its beta testing.
Long-term, we should consider making HERE in debugs() at level 2 or
higher an automatic/default behavior. This will require some work,
but I
think it is possible, and I think it can even accommodate existing
debugs() statements (with or without HERE) _without_ changing them.
It's hairy at best, if it can even be done, as it depends on cpp
predefined macro magic (__LINE__, __FILE__ etc).
Good point. Testing needed.
Amos
