AUDIT: r17840 - gnucash/trunk/src/register/ledger-core - Register: Add additional debugging output during register cleanup. Also rename a variable for clarity and to match typical usage in the rest of the ledger code.

[Charles Day]

On Fri, Jan 23, 2009 at 12:31 PM, Derek Atkins <warlord at mit.edu> wrote:

> Hi,
>
> Charles Day <cedayiv at gmail.com> writes:
>
> > On Thu, Jan 22, 2009 at 11:02 AM, Derek Atkins <warlord at mit.edu> wrote:
> >
> >     Charles Day <cedayiv at cvs.gnucash.org> writes:
> >
> >     > Register: Add additional debugging output during register cleanup.
> Also
> >     rename a variable for clarity and to match typical usage in the rest
> of
> >     the ledger code.
> >     > BP
> >
> >     Why is this something that should be back ported?
> >
> > Do you not want new debugging output backported? I had to add this to
> > troubleshoot the register with a user (bug 426111), as he could randomly
> > reproduce the problem himself but could not describe how to make it
> happen for
> > others. It would be nice to have this change released so that users can
> > produce this output on request with --log gnc.ledger=debug if/when we
> can't
> > reproduce their problem.
>
> I think it depends where we are in the lifecycle of the stable
> release.  Right now I'd consider 2.2 "nearing its end", and really we
> should limit changes to true bug fixes, and even then we should be
> conservative about what it is that we're fixing.  Simultaneously I
> think we should seriously think about the 2.3/2.4 lifecycle.
>

Do we have a timeframe for 2.3/2.4? If these debugging messages might help
me fix register problems then it could be worth adding them in 2.2, because
then users will be better able to help me. That could help get some register
bugs fixed in trunk before 2.3/2.4.  Otherwise register bugs may just
continue to sit around.  I'm definitely not saying to backport the actual
fixes for 2.2, but at least backporting the debugging messages may help keep
fixes coming.

I'm just sort of wary of getting into the situation where there's a new
stable release and fixes will be quarantined in trunk again. So I'd like to
get some register fixes done before the next stable release goes out, and
these debugging messages may help.


> > As for the variable name change, that's strictly optional, but the old
> name
> > was confusing when read in context with all the other functions in that
> file.
> >
> > Incidentally, I just committed a bunch of changes related to doxygen. I
> guess
> > those don't really have to be backported if documentation is never
> produced
> > from branches, though it seems innocuous.
>
> Innocuously-seeming changes often come back to bite us.  Just look
> at the one-line change put into 2.2.8 that causes "close invoice"
> to crash!
>

Agreed, however this particular changeset (r17841) doesn't change any
functionality - it is all edits to comments except in two spots where a
function declaration in the .h file had a different parameter name from in
the .c file. Simple typos -- I just made them match.  Anyway that changeset
is just for documentation, so no problem, no need to backport it. There will
be another doxygen changeset committed soon. I won't mark that one for
backport.


> I would recommend caution and restraint about backporting changes.
> If it's not a crasher, if it's not causing data loss or data corruption,
> then it probably doesn't need to get backported.
>
> In terms of adding debugging output....  That's definitely a tough
> call.  If the problem is so subtle that we can't figure it out then
> it's probably minor enough that I would err on the side of "wait for
> 2.3".
>

I think the debugging code is a very safe change. The code doesn't even run
unless the --log option is set. If you look at r17837 and r17838, all you'll
see are new ENTER(), DEBUG() and LEAVE() statements. For r17839, if the
variable name change worries you then don't backport it, that's fine.
There's only one new debugging message added in r17839 anyway.


>
> And yes, documentation is built off trunk, not 2.2.
>
> -derek
>
> --
>       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
>       Member, MIT Student Information Processing Board  (SIPB)
>       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
>       warlord at MIT.EDU                        PGP key available
>

-Charles