[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Feature request/ideas - final patch
From: |
Frank Hemer |
Subject: |
Re: Feature request/ideas - final patch |
Date: |
Wed, 6 Apr 2005 22:37:46 +0200 |
User-agent: |
KMail/1.5.1 |
> |> 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.
I have adapted most of the broken code, so again all tests pass. Unfortunately
RCS_tag2rev() needs a special handling because it is responsible for adding
rev. numbers (with magic) to the RCS file. I also had to fix part of the old
code because for ex. RCS_nodeisbranch didn't properly detect a branch with
plain revision numbers which lead to many workarounds. Your fix of admin.c
broke parts of the existing code that wasn't tested by sanity.sh (symbolic
tags with admin -b), this is fixed too. I started to add some more tests but
still this is not sufficiant. I think it might make sense to put an
intermediate commit to the newtags branch to make development less complex.
Is it ok to send intermediate patches (tests pass but more work to be done)?
Regards
Frank