Re: [PATCH v2] Add svnrdump

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jul 15, 2010, 12:01 UTC
Message-ID: <20100715120143.GE22574@debian>
In-Reply-To: <20100714172429.GC25861@ted.stsp.name>

Hi Stefan,

Stefan Sperling writes:

Show 12 quoted lines

> > This isn't in the dump-load-format spec document (atleast afaik), and
> > it's very hard to get this right (yes, I tried). Moreover, it's very
> > ungratifying to have a few extra newlines (reverse engineered from
> > `svnadmin dump`) printed at the end of 10+ hrs of work; yes, that's
> > what I estimate it'll take to fix this.
> 
> Well, it would be really nice to have.
> Details like this are time sinks, I know. But it pays off.
> You don't have to do it right away. We can file an issue so we don't
> forget about fixing it before 1.7 release.
> If necessary, feel free to adjust the output of svnadmin dump a little
> if that makes it easier for svnrdump to produce matching output.

I think the latter is certainly an option. We definitely need to fix the dump-load-format spec to show everything.

> > gawk '$0 !~ "Prop-delta: true|Text-delta-base-|sha1|Text-copy-source-|^-$" && $0 ~ "^+|^-" { print; }'
> 
> Fine for testing. But I still think the end-result should look just
> like svnadmin dump, if possible. That would make testing even easier.

Right. We can use the same test suite and maintenance would become infinitely easier.

Show 8 quoted lines

> > > Please get rid of all global variables in svnrdump.c:
> > Will do. I'm waiting for commit access, because I don't want to make
> > un-versioned edits to the file that I cannot track or revert in
> > future.
> 
> What about using git until then? It does not matter which state you
> initially import into the Subversion repository. But well, whatever
> works for you is best.

Oh, I didn't think it would take this long for my account to get activated. I'll consider using Git to stage for now because I don't want to delay the response to your review.

-- Ram