[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [stellation-res] Parsing location spec - patch 34576
|
Jonathan,
I've looked over the patches for #34576, and they basically look fine.
A couple of points about patch content:
1. You've defined LOCATION_OPTION_SEPARATOR as '+' in the patch.
I assume we would want to keep the value as ':' until a different value is
agreed?
2. The posted patches include some Firebird-related changes (e.g. changing
from "firebird" to "firebirdsql" in
stellation.cli.data.Location.java). These seem reasonable
too, but perhaps all the Firebird-enabling changes should go in as a single
Bugzilla item?
There are also some stray comment headers included in the patch (no harm
done, but
a bit extraneous).
Some points about patch process:
A. The patches were apparently created at varying folder nesting levels
within the five projects being patched (cli, core, remote, scm.model,
unittest). This made it a bit confusing to review the patches: I had to
figure which folder or individual file was the base node for each given
patch. Mark and I generate patches using the project folder as the base
node; this produces project-relative paths within the patch file, and
facilitates applying or reviewing the patch.
B. The Bugzilla attachment had the name 'attachment.cgi' even though it was
a zip file. A similar issue occurred with Bugzilla 34560 (which I just
approved,
via Bugzila): the attachment for 34560 was a single text file (patch file) that
was also named 'attachment.cgi'.
Mark has started zipping every patch using the bugzilla number as the patch
file name
(e.g. 34560.zip would be a zip file containing a single text file named
cli.patch).
In a recent patch zip, Mark also included the Bugzilla number as the path
prefix.
For #34576, using these conventions, the attachment would be:
Archive: 34576.zip
34576/cli.patch
34576/core.patch
34576/remote.patch
34576/scm-model.patch
34576/unittest.patch
I like this approach, and will use it for my future patches. It facilitates
keeping a single 'patches' directory with all patches neatly organized into
folders by Bugzilla number, and also makes it easier to apply or review
patches.
Regards,
Jim
At 06:40 AM 3/11/2003, Jonathan Gossage wrote:
As a follow-up to my earlier post on this subject I have created a patch
that uses a symbolic definition for the database location separator. The
patch still uses the problem definition of ':' but this is easily changed
once we settle on a resolution to the Firebird problem.
The patch is attached to Bugzilla #34576. Could someone take a look at it
please?
Regards
Jonathan
--
Jim Wright, IBM T.J. Watson Research Center
*** The Stellation project: Advanced SCM for Collaboration
*** http://www.eclipse.org/stellation
*** Work Email: jwright@xxxxxxxxxxxxxx ------- Personal Email:
jim.wright@xxxxxxx
