Re: svn commit: r805287 - in /continuum/branches/continuum-1.3.x

[brettporter]

Hi Marica,

There seem to be some unrelated changes in this rev? See below:

On 18/08/2009, at 2:32 AM, ctan@... wrote:

> Author: ctan
> Date: Tue Aug 18 06:32:41 2009
> New Revision: 805287
>
> URL: http://svn.apache.org/viewvc?rev=805287&view=rev
> Log:
> [CONTINUUM-2194] use plexus-spring in data management cli
>
> Added:
>   continuum/branches/continuum-1.3.x/continuum-api/src/main/java/org/
> apache/continuum/utils/ProjectSorter.java
>   continuum/branches/continuum-1.3.x/continuum-api/src/test/java/org/
>   continuum/branches/continuum-1.3.x/continuum-api/src/test/java/org/
> apache/
>   continuum/branches/continuum-1.3.x/continuum-api/src/test/java/org/
> apache/continuum/
>   continuum/branches/continuum-1.3.x/continuum-api/src/test/java/org/
> apache/continuum/utils/
>   continuum/branches/continuum-1.3.x/continuum-api/src/test/java/org/
> apache/continuum/utils/ProjectSorterTest.java

Why do these classes need to be removed for the data management? If  
so, should they have been moved with svn mv instead of adding/removing?

>   continuum/branches/continuum-1.3.x/continuum-data-management/data-
> management-cli/src/main/java/org/apache/maven/continuum/management/
> util/
>   continuum/branches/continuum-1.3.x/continuum-data-management/data-
> management-cli/src/main/java/org/apache/maven/continuum/management/
> util/PlexusApplicationContextDelegate.java
>   continuum/branches/continuum-1.3.x/continuum-data-management/data-
> management-cli/src/main/java/org/apache/maven/continuum/management/
> util/PlexusFileSystemXmlApplicationContext.java

Are these new classes you created or are they extracted from another  
source? They seem to be missing a license in this commit.

- Brett

[Marica Tan-2]

On Mon, Aug 24, 2009 at 4:48 AM, Brett Porter <brett@...> wrote:

> Hi Marica,
>
> There seem to be some unrelated changes in this rev? See below:
>
> On 18/08/2009, at 2:32 AM, ctan@... wrote:
>
>  Author: ctan
>> Date: Tue Aug 18 06:32:41 2009
>> New Revision: 805287
>>
>> URL: http://svn.apache.org/viewvc?rev=805287&view=rev
>> Log:
>> [CONTINUUM-2194] use plexus-spring in data management cli
>>
>> Added:
>>
>>  continuum/branches/continuum-1.3.x/continuum-api/src/main/java/org/apache/continuum/utils/ProjectSorter.java
>>  continuum/branches/continuum-1.3.x/continuum-api/src/test/java/org/
>>
>>  continuum/branches/continuum-1.3.x/continuum-api/src/test/java/org/apache/
>>
>>  continuum/branches/continuum-1.3.x/continuum-api/src/test/java/org/apache/continuum/
>>
>>  continuum/branches/continuum-1.3.x/continuum-api/src/test/java/org/apache/continuum/utils/
>>
>>  continuum/branches/continuum-1.3.x/continuum-api/src/test/java/org/apache/continuum/utils/ProjectSorterTest.java
>>
>
> Why do these classes need to be removed for the data management? If so,
> should they have been moved with svn mv instead of adding/removing?

Yes, I had a problem with applying the patch that's why it was added/removed
instead of mv.

The data management is using ProjectSorter. I need to set the dependency
scope of continuum-commons to test only because it's loading classes which
needs to connect to the database while loading beans from application
context.

>
>  continuum/branches/continuum-1.3.x/continuum-data-management/data-management-cli/src/main/java/org/apache/maven/continuum/management/util/
>>
>>  continuum/branches/continuum-1.3.x/continuum-data-management/data-management-cli/src/main/java/org/apache/maven/continuum/management/util/PlexusApplicationContextDelegate.java
>>
>>  continuum/branches/continuum-1.3.x/continuum-data-management/data-management-cli/src/main/java/org/apache/maven/continuum/management/util/PlexusFileSystemXmlApplicationContext.java
>>
>
> Are these new classes you created or are they extracted from another
> source? They seem to be missing a license in this commit.
>

PlexusApplicationContextDelegate was taken from plexus-spring 1.2.
PlexusFileSystemXmlApplicationContext is newly created.


>
> - Brett
>
>

[brettporter]

On 23/08/2009, at 7:46 PM, Marica Tan wrote:

>>
>> Why do these classes need to be removed for the data management? If  
>> so,
>> should they have been moved with svn mv instead of adding/removing?
>
>
> Yes, I had a problem with applying the patch that's why it was added/
> removed
> instead of mv.

In future, I'd do the mv then put the corrected copy over the top.

>
> The data management is using ProjectSorter. I need to set the  
> dependency
> scope of continuum-commons to test only because it's loading classes  
> which
> needs to connect to the database while loading beans from application
> context.

I wouldn't normally think this is a good idea (especially if others  
depend on these libraries). You can set those beans to lazy-init if  
you need to... but for now this is probably fine. The modules need a  
definite clean up anyway.

>
>
>>
>> continuum/branches/continuum-1.3.x/continuum-data-management/data-
>> management-cli/src/main/java/org/apache/maven/continuum/management/
>> util/
>>>
>>> continuum/branches/continuum-1.3.x/continuum-data-management/data-
>>> management-cli/src/main/java/org/apache/maven/continuum/management/
>>> util/PlexusApplicationContextDelegate.java
>>>
>>> continuum/branches/continuum-1.3.x/continuum-data-management/data-
>>> management-cli/src/main/java/org/apache/maven/continuum/management/
>>> util/PlexusFileSystemXmlApplicationContext.java
>>>
>>
>> Are these new classes you created or are they extracted from another
>> source? They seem to be missing a license in this commit.
>>
>
> PlexusApplicationContextDelegate was taken from plexus-spring 1.2.
> PlexusFileSystemXmlApplicationContext is newly created.

We can't use plexus-spring 1.2?

- Brett

[Marica Tan-2]

On Mon, Aug 24, 2009 at 8:27 AM, Brett Porter <brett@...> wrote:

>
> On 23/08/2009, at 7:46 PM, Marica Tan wrote:
>
>
>>> Why do these classes need to be removed for the data management? If so,
>>> should they have been moved with svn mv instead of adding/removing?
>>>
>>
>>
>> Yes, I had a problem with applying the patch that's why it was
>> added/removed
>> instead of mv.
>>
>
> In future, I'd do the mv then put the corrected copy over the top.
>
>
>> The data management is using ProjectSorter. I need to set the dependency
>> scope of continuum-commons to test only because it's loading classes which
>> needs to connect to the database while loading beans from application
>> context.
>>
>
> I wouldn't normally think this is a good idea (especially if others depend
> on these libraries). You can set those beans to lazy-init if you need to...
> but for now this is probably fine. The modules need a definite clean up
> anyway.
>
>
>>
>>
>>>
>>> continuum/branches/continuum-1.3.x/continuum-data-management/data-management-cli/src/main/java/org/apache/maven/continuum/management/util/
>>>
>>>>
>>>>
>>>> continuum/branches/continuum-1.3.x/continuum-data-management/data-management-cli/src/main/java/org/apache/maven/continuum/management/util/PlexusApplicationContextDelegate.java
>>>>
>>>>
>>>> continuum/branches/continuum-1.3.x/continuum-data-management/data-management-cli/src/main/java/org/apache/maven/continuum/management/util/PlexusFileSystemXmlApplicationContext.java
>>>>
>>>>
>>> Are these new classes you created or are they extracted from another
>>> source? They seem to be missing a license in this commit.
>>>
>>>
>> PlexusApplicationContextDelegate was taken from plexus-spring 1.2.
>> PlexusFileSystemXmlApplicationContext is newly created.
>>
>
> We can't use plexus-spring 1.2?
>

plexus-spring 1.2 does not have a PlexusFileSystemXmlApplicationContext so I
have to create one

>
> - Brett
>
>

[brettporter]

On 23/08/2009, at 8:47 PM, Marica Tan wrote:

>>>>
>>> PlexusApplicationContextDelegate was taken from plexus-spring 1.2.
>>> PlexusFileSystemXmlApplicationContext is newly created.
>>>
>>
>> We can't use plexus-spring 1.2?
>>
>
> plexus-spring 1.2 does not have a  
> PlexusFileSystemXmlApplicationContext so I
> have to create one

I meant for the first one :)

- Brett

[Marica Tan-2]

On Mon, Aug 24, 2009 at 9:38 AM, Brett Porter <brett@...> wrote:

>
> On 23/08/2009, at 8:47 PM, Marica Tan wrote:
>
>
>>>>>  PlexusApplicationContextDelegate was taken from plexus-spring 1.2.
>>>> PlexusFileSystemXmlApplicationContext is newly created.
>>>>
>>>>
>>> We can't use plexus-spring 1.2?
>>>
>>>
>> plexus-spring 1.2 does not have a PlexusFileSystemXmlApplicationContext so
>> I
>> have to create one
>>
>
> I meant for the first one :)
>

ah :) methods are all protected that's why I copied the class instead.

>
> - Brett
>
>