Re: [cdt-patch] ToolReference refactor

[Sean Evoy <sevoy@xxxxxxxxxx>]

Jeremiah,
Interesting that I was just working
on the ToolReference myself so this comes at a good time. In 1.2, I pretty
much used the tool reference as a place-holder in the .cdtbuild file to
hang onto the overridden option settings that the user changed through
the UI. I also used it in the manifest to change specific option settings
on a per-configuration basis for tools defined in (or inherited by) the
config's target.

In this release though, I was planning
on using the reference to hang onto overridden tool commands, so at least
the getToolCommand method was going to be used. There was also a request
to make the manifests more compact, so we decided that targets could also
have tool references. Tools can be defined at the same "level"
in the manifest as targets, so it makes sense that we could define a GCC
compiler 1 time (which would eliminate a lot of duplication/error in the
manifest) and have each target that uses that compiler have a reference
to it.

With that in mind, I took a look at
the patch and it looks like it would have 0 impact on the work I want to
do. The implementation class can store the overridden tool command (and
any other tool related settings the user might be allowed to change in
the future). I had planned to replace the owner in the ToolReference with
the base class IConfiguration shares with ITarget, but I suppose I could
just as easily create another implementation class that has a ITarget as
the owner. It might actually be a cleaner implementation.

I have not looked at the JUnit test
implications, but I want to overhaul those anyway, so maybe I'll do that
next. It is a good proposal and I can see some immediate advantage for
my work, so I will apply it to a clean workspace, give it a more thorough
testing and get back to you. 

Thanks.

Sean Evoy
Rational Software - IBM Software Group
Ottawa, Ontario, Canada

"Lott, Jeremiah" <jeremiah.lott@xxxxxxxxxxx>
Sent by: cdt-patch-admin@xxxxxxxxxxx

02/18/2004 06:27 PM

Please respond to
cdt-patch

To	<cdt-patch@xxxxxxxxxxx>
cc
Subject	[cdt-patch] ToolReference refactor

In working on my prototype I came across a small refactor
to managed
build I thought might be generally useful.  It regards tool reference.
Currently the class
org.eclipse.cdt.managedbuilder.internal.core.ToolReference is used to
"wrap" a tool instance.  Many of the methods are simple
delegators, some
override the values based on the .cdtbuild file to reflect user
specified changes.  What I did was make an abstract class in
managedbuilder.core that implements ITool and delegates all of its
methods to a wrapped ITool instance.  I then changed the existing
class
to extend this abstract class, and removed the simple delegators as they
are now unnecessary.  I also extended the abstract class in my
prototype, but I'm not submitting that.  This doesn't have any user
visible effect, it just makes it easier for people like me to make
customizations to managed build.

The main thing I'm not sure about is the names.  I called my abstract
class org.eclipse.cdt.managedbuilder.core.ToolReference.  I renamed
the
existing ToolReference class to
org.eclipse.cdt.managedbuilder.internal.core.ConfigToolReference.

Sean (and anyone else interested in managed build), do you think this
refactoring is useful?  If you want different names for the classes,
just let me know and I'll regenerate the patch.  If you don't think
this
is generally useful, so be it.   I'll just withdraw the patch.

I'm having problems getting the test suite to run.  I'll try and fix
that tomorrow, but I wanted to at least get this out there for review.

  Jeremiah Lott
  TimeSys Corporation

Attachment: tool-reference-refactor.patch
Description: Binary data