Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [stellation-res] Proposed change recorded in bugzilla

Mark and colleagues --

[Note: this was written Wednesday pm, but didn't get sent somehow
-- my bad.]

At 04:34 PM 2/4/2003, Mark C. Chu-Carroll wrote:
I've just finished a fairly large change as a part of my effort to
do some code cleanup. The changed version now passes tests, both local
and remote. Since it's significant, I'd like to have at least two
people double-check it for any stupidity. Jonathan and Jim, can you
please each take a look at it? (Anyone else who'd like to is more
than welcome to also take a look, and send me comments.)

The bug number is 30515, and the changes are attached as a zipfile
containing patches for each of the modified projects in attachment 3288.

The changes cause one error in scm.model, which will require some
cascading changes to fix. If you guys agree the basic change is
valid, I'll consult with Jim on fixing that.

I've checked the patch set and the changes are good.

My test procedure:
1.  Create a virgin Eclipse workspace.  Install all required projects including
     3rd party libs.
2.  Check out the latest source from our CVS repo on eclipse.org
3.  Apply the patches to the fresh source using standard Eclipse methods
(Package View - highlight project name - right click - Compare With > Patch)
4.  Update class paths in Eclipse and rebuild all.

5. Clean out my previous Stellation server files (in usr/local/bin and usr/local/lib/stellation). 6. Rebuild and install the Stellation server using ...stellation.core/build-core.xml. Note: I used my current version of build-core.xml, given to Mark for testing earlier this week, because it's simpler/more straightforward than the prior build-core.xml (and I understand my version better :>) The new version is perhaps moot, since I've just confirmed that the Eclipse-generated build.xml files can be used to build the whole shebang from the command line in one go (excluding server installation -- so maybe it's not moot. Moving on...)

7. Run the remote tests using $ant .../stellation.core/test arg arg arg..... as documented in the latest version of .../stellation.core/doc/dev-install.html --- which nobody but Mark has
     currently; let's fix that rsn.
>>>     Remote tests passed - no problems

8, Run the local tests per above. There were a few configuration glitches (must use a password of 'password' the way the tests are currently written), but then everything worked fine.

I'd like to hear Jonathan's opinion, but from where I type, it's a go.

*** Comments after reviewing the API changes
(FYI, I used Package Explorer - right click on project - Compare With > Latest From Repository to find what changed --- worked fairly well, but I *really* want a Stellation method-aware diff !!!!)

The API changes are a clear improvement.  It's better to pass project names
etc. explicitly (rather than using hidden state),  and support for raw ints
(rather than Integers, except where absolutely needed) for version numbers
is also good.

The added exceptions and exception handling is much needed.
Thank you for adding it ...
Thank you for adding it....
Thank you.....

The simplified ctors (e.g. as used in cli.workspace.Merge) are also nice -
less typing, fewer typos.

repos.util.MergeManager looks a lot more robust now that mergeChanges
includes the project name, branch name and branch version in the resulting
BranchImage (I'm a bit surprised it worked before).  Ditto for
RetrievalManager.retrieveProjectVersion (where the removal of the
'desiredTypes' method parameter seems like a good simplification --- I never
used it..)

I note that ...util.TriggerScript methods have been changed to explicitly
pass project and branch names and an int version, as well as the
branchImage.  Seems good.  Do you know if there were/are cases where the bi
was not properly initialized with project/branch/version info?

About the many added comments, all I can say is "No comment .... not!"

In remote.server.UpdateHandler -- good catch; it appears the project name
was previously omitted from the BranchSpecList returned by getParents.

The changes to stellation.scm.model all look fine.  I fixed the broken bit
in model2.ScmArtifactory.buildParentsList. and everything compiles and runs
ok (Well, as well as it used to -- a serious overhaul is needed here).

FYI, The fix to buildParentsList is 2 lines, added at the beginning of the method:

    Element documentElement = document.getDocumentElement();
    String projectName = documentElement.getAttribute("project");
    ....

--------------------------------

I did not review the changes to stellation.unittest; let me know if you think that's necessary.

Otherwise (and unless Jonathan's eagle eye finds something) --
please check it in. I have some fixes to make to scm.model now, but they're well worth it.

- Jim

--
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



Back to the top