Snjezana:
Thanks for your input. There certainly was no way we'd copy all 5 of
their classes (including TomcatPlugin! ha!), so for GA it was the right
move to make.
I tend to agree with Max, though, in regards to isolating the
functionality and limiting dependencies. I hate adding dependencies and
I hate I have so many as it is ;) I wish I could get my dependency list
down to next to nothing!
Anyway, thanks for the patch in the first place. I didn't mean to jump
on you like that and certainly not to discourage you from patching my
code when needbe.
Thanks so much for the help =]
Snjezana Peco wrote:
There are three reasons why I decided to add a dependency:
- we would have to copy at least 5 classes
(TomcatSourcePathComputerDelegate, IModuleVisitor, ModuleTraverser,
TomcatPlugin, org.eclipse.jst.server.tomcat.core.internal.Trace ...)
- I have entered bug
https://bugs.eclipse.org/bugs/show_bug.cgi?id=233110 against wtp. When
they fix this bug, we will revert changes (3 lines)
- I don't like to have the same code in multiple places
I don't see any problem with adding a new dependency because JBoss
Tools already depends on wtp that includes the Tomcat adapter.
I have seen that the ServerSourcePathComputerDelegate class is a
duplicate of GenericServerSourcePathComputerDelegate, but have thought
that the reason for that was removing the
org.eclipse.jst.server.generic.jboss from JBDS what makes sense for me.
Snjeza
Rob Stryker wrote:
> Max:
>
> I just found this commit today (I don't browse commit logs much)...
> Is it better to depend on a new plugin or to copy the class in
> question to a .xpl package?
> I personally would have coppied the class. Snjeza decided to add a
> dependency.
>
> Thoughts? I personally find it very silly that the AS adapter would
> depend on the tomcat adapter.
>
> jbosstools-commits(a)lists.jboss.org wrote:
>> Author: snjeza
>> Date: 2008-05-20 17:21:12 -0400 (Tue, 20 May 2008)
>> New Revision: 8244
>>
>> Modified:
>>
>>
branches/jbosstools-2.1.x/as/plugins/org.jboss.ide.eclipse.as.core/META-INF/MANIFEST.MF
>>
>>
>> branches/jbosstools-2.1.x/as/plugins/org.jboss.ide.eclipse.as.core/plugin.xml
>>
>> Log:
>> JBIDE-2215 Unable to add User Library as Source Lookup Path to
>> AS/EAP server
>>
>> Modified:
>>
branches/jbosstools-2.1.x/as/plugins/org.jboss.ide.eclipse.as.core/META-INF/MANIFEST.MF
>>
>> ===================================================================
>> ---
>>
branches/jbosstools-2.1.x/as/plugins/org.jboss.ide.eclipse.as.core/META-INF/MANIFEST.MF
>> 2008-05-20 21:09:49 UTC (rev 8243)
>> +++
>>
branches/jbosstools-2.1.x/as/plugins/org.jboss.ide.eclipse.as.core/META-INF/MANIFEST.MF
>> 2008-05-20 21:21:12 UTC (rev 8244)
>> @@ -19,7 +19,8 @@
>> org.eclipse.jst.common.project.facet.core,
>> org.eclipse.wst.xml.core,
>> org.jboss.ide.eclipse.archives.core,
>> - org.apache.ant
>> + org.apache.ant,
>> + org.eclipse.jst.server.tomcat.core
>> Eclipse-LazyStart: true
>> Export-Package: org.jboss.ide.eclipse.as.core,
>> org.jboss.ide.eclipse.as.core.extensions.archives,
>>
>> Modified:
>> branches/jbosstools-2.1.x/as/plugins/org.jboss.ide.eclipse.as.core/plugin.xml
>>
>> ===================================================================
>> ---
>> branches/jbosstools-2.1.x/as/plugins/org.jboss.ide.eclipse.as.core/plugin.xml
>> 2008-05-20 21:09:49 UTC (rev 8243)
>> +++
>> branches/jbosstools-2.1.x/as/plugins/org.jboss.ide.eclipse.as.core/plugin.xml
>> 2008-05-20 21:21:12 UTC (rev 8244)
>> @@ -253,7 +253,7 @@
>> name="JBoss Application Server Startup Configuration"
>> public="true"
>>
>>
sourceLocatorId="org.eclipse.jdt.launching.sourceLocator.JavaSourceLookupDirector"
>>
>> -
>>
sourcePathComputerId="org.jboss.ide.eclipse.as.core.serverSourcePathComputer"/>
>>
>> +
>>
sourcePathComputerId="org.eclipse.jst.server.tomcat.core.sourcePathComputer"/>
>>
>> <launchConfigurationType
>>
>>
delegate="org.jboss.ide.eclipse.as.core.server.internal.launch.DeployableLaunchConfiguration"
>>
>>
>> id="org.jboss.ide.eclipse.as.core.server.stripped.launchConfiguration"
>> @@ -261,7 +261,7 @@
>> name="Stripped Server Launch Configuration"
>> public="false"
>>
>>
sourceLocatorId="org.eclipse.jdt.launching.sourceLocator.JavaSourceLookupDirector"
>>
>> -
>>
sourcePathComputerId="org.jboss.ide.eclipse.as.core.serverSourcePathComputer"/>
>>
>> +
>>
sourcePathComputerId="org.eclipse.jst.server.tomcat.core.sourcePathComputer"/>
>>
>> <launchConfigurationType
>>
>>
delegate="org.jboss.ide.eclipse.as.core.server.internal.launch.TwiddleLaunchConfiguration"
>>
>>
>> id="org.jboss.ide.eclipse.as.core.server.twiddleConfiguration"
>>
>> _______________________________________________
>> jbosstools-commits mailing list
>> jbosstools-commits(a)lists.jboss.org
>>
https://lists.jboss.org/mailman/listinfo/jbosstools-commits
>>
>
> _______________________________________________
> jbosstools-dev mailing list
> jbosstools-dev(a)lists.jboss.org
>
https://lists.jboss.org/mailman/listinfo/jbosstools-dev