[jbosstools-dev] Re: Weird code in Seam runtime management...

[Max Rydahl Andersen]

>> I stepped over some code I don't understand why is needed in Seam 
>> runtime management.
>>
>> SeamRuntimeListConverter1 (weird class name, why an 1 at the end ?), 
> Take a look at ICompletionProposalExtension, 
> ICompletionProposalExtension2, ICompletionProposalExtension3, 
> ICompletionProposalExtension4, ICompletionProposalExtension5

Eh yes - so where is SeamRuntimeListConverter ?

The numbering schema eclipse uses are weird, but besides that this is 
only because they need to have backward compatible interfaces.

SeamRuntimeListConverter is a implementation detail, hence doesn't need 
these constraints.

>> seem to do some really weird conversion between maps and strings on 
>> the form
> really weird have XML for simple map or even serialization for it.
>>
>> "name|some Name|version|1123123|homeDir|somedir|default|true"
>>
>> ...why are we storing such weird strings in preferences ?
>> (Is that normal for eclipse preferences ? 
> That's the simple way to serialize list of runtimes. Look at 
> .metadata\.plugins\org.eclipse.core.runtime\.settings | used everywhere 
> for lists and especially for General/Apperance preferences

horrible...it's 2007 and that is the best they can come up with ;)

>> That looks very unmaintainable)
> unmaintainable?

So let me see what happens if I put a | in the version or name ;)

>> Why isn't the info about what runtime is default a single named 
>> preference instead of maintained on each runtime ?
> We discussed it before, we have to have default version for 1.2, 20, 
> etc. because default seam-runtime is depends on version selected for 
> facet in Seam Web Project wizard

Ah ok. But then having a seam.version.2.default or something would be 
better. anyway not a big issue; I just found it to be redundant.

>> And why are these classes in a package called 
>> org.seam.core.project.facet ? These things are and should be 
>> independent of WTP facets (and SeamRuntimeListConverter1 sound like 
>> something that should be inside the internal package)

> Because in previous releases have no preferences for project with seam 
> feature it was just a feature now we have runtime in both projects 
> faceted and others. just need refactor it to new packages

sure - but runtime never were meant to be facet specific IMO. Changing 
it would be good; but also remember that if we have internal packages 
then things should go there before going into the "public" packages.

Actually since none of our plugins is intended (currently) to be 
utilized by anyone else to be extended everything should be in internal.

/max