[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Feature request/ideas - final patch

From: Derek Price
Subject: Re: Feature request/ideas - final patch
Date: Wed, 06 Apr 2005 16:04:31 -0400
User-agent: Mozilla Thunderbird 1.0 (Windows/20041206)

Hash: SHA1

Frank Hemer wrote:

|> Frank,
|> I've committed a few new changes and the tests are passing again,
|>  including your new tag-ext tests.
|> I also added a FIXME comment or two.  Please take a look,
|> especially at the one in RCS_getprevious.
| Well, according to my comment above your FIXME (RCS_getprevious)
| the code does exactly what your fixme suggests;-)

Yep.  Sorry.  I've removed the FIXME.

|> I've spotted several errors and omissions so far and have cause
|> to wonder if the only reason tag-ext passes currently is that
|> testing is not complete.  Please see what you can do about adding
|> more tests.
| Could you provide more details what exactly fails? Regarding your
| changes several assumptions are broken. Did you test with my
| original patch?

I tested with your original patch.  It passed the tests you provided.
I still thought your code was overly complicated.  For one thing, you
had a bunch of special casing for the .trunk case which I thought was
unnecessary.  I started removing some of it from one file, admin.c.
One of the basic changes that made this possible was simply returning
a branch number (like "1", or "2") for the trunk, or maybe it was the
head revision of the trunk, from RCS_gettag, and returning the latest
revision on the trunk from RCS_tag2rev.

I also started removing some obfuscations in the RCS_* and some other
functions in rcs.c, including several of your new functions, like
translate_tag.  I also added assertions that some of the assumptions
you missed were true, for reference, as I simplified the functions
based on these assumptions.

I managed to verify that the old and new tests referencing the admin
command still pass, but some of my changes to the rcs.c functions have
broken your new code for other CVS commands that are probably still
special casing and/or depend on the old behavior of the new functions.

Please take a look at the diffs and the new comments and assertions
now on the "newtags" branch in the CVS repository and see if you can
complete the revisons I started to your patch.  Also, your work needs
more complete comments for final acceptance accepted.


Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org


reply via email to

[Prev in Thread] Current Thread [Next in Thread]