Tobias Ringström <tobias@ringstrom.mine.nu> writes:
> >Author: david
> >Date: Sun Sep 26 23:11:55 2004
> >New Revision: 11143
> >
> >Modified:
> > branches/1.0.x/packages/rpm/redhat-8+/doc.patch
> > branches/1.0.x/packages/rpm/redhat-8+/subversion.spec
> >
> Um, you committed this to the 1.0.x branch...
His log message seemed to indicate that this was intentional:
| r11143 | david | 2004-09-26 23:11:55 -0500 (Sun, 26 Sep 2004) | 12 lines
|
| Note: The 1.1.0 branch and trunk don't seem to have this problem so I'm just
| fixing it here.
|
| * packages/rpm/redhat-8+/doc.patch : Fix book Makefile patch to work again
| after breakage that happened right before 1.0.7 was release. This also
| fixes the shell-script bug where the number was not being inserted
| correctly because of the parentheses instead of braces.
|
| * packages/rpm/redhat-8+/subversion.spec : Fix specfile to output
| the version to the book version.xml instead of the release.
But, a separate question is whether this change had a confirming +0
vote from some other committer, as required by our release-branch
review policies. I don't see any other committer listed in the log
message, so it looks like David approved this solo.
David, I just reviewed it now, intending to save you the trouble of
having revert the change, then get a +0 from someone, then
reapply... But I can't offer a +0 yet, because I have some questions.
Regarding the change:
I can see one place where you substituted curly braces for parens, in
doc.patch. But you left other parens untouched, even though it seems
to me they're being used in the same way? Why not change them all? I
think the current inconstency is both likely to cause confusion, and
likely to cause some sort of subtle bug later on when one style is
cut-and-pasted into a context where the other style would have been
needed. It would be better if there were a comment explaining why the
difference, or (if possible) one style used consistently?
Regarding the log message:
Don't you also want to say that you removed the word "Draft" from the
book's revision label, in doc.patch? And you might want to fix this
ambiguity:
| * packages/rpm/redhat-8+/subversion.spec : Fix specfile to output
| the version to the book version.xml instead of the release.
It sounds like the change is to output the version "to version.xml"
instead of "to the release" -- when what's actually happening is
you're outputting the version, instead of the release, to version.xml.
(Btw, I don't know the difference between the "version" and the
"release", so I wasn't able to review that bit of the change.)
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Sep 27 19:21:06 2004