RE: [stellation-res] Parsing location spec - patch 34576

["Jonathan Gossage" <jgossage@xxxxxxxx>]

> -----Original Message-----
> From: stellation-res-admin@xxxxxxxxxxxxxxx
> [mailto:stellation-res-admin@xxxxxxxxxxxxxxx]On Behalf Of Jim Wright -
> IBM Research
> Sent: Tuesday, March 11, 2003 6:27 PM
> To: stellation-res@xxxxxxxxxxxxxxx
> Subject: 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
I am having a lot of problems with the patch mechanism. I created .zip files
with a similar naming convention and I don't know how they got changed to
.cgi. Probably a misuse of Bugzilla but I find the patch submission process
in Bugzilla a bit obscure.

The reason that the patches were created at different levels was that I have
other modifications that I did not want to include so I had to try and get
very fine-grained. I thought that I had backed out the use of '+' and the
definition of firebirdsql before making the patch. I was intending to submit
separate patches for those changes.

In general I am finding the process of creating and sending patches to be
very error prone. Hopefully when we can move to Stellation where we can use
branches instead of patches this will be easier.

Regards

Jonathan