Skip to main content

Please review fix for issue 104

3 replies [Last post]
Anonymous

Hi Vladimir,

please review fix for [Issue 104]: "REGRESSION: some original RI's startup
scripts don't work with current version of ME Framework". The changesets:
http://fisheye4.cenqua.com/changelog/cqme/branches/users/skavas/secproxy...
http://fisheye4.cenqua.com/changelog/cqme/branches/users/skavas/secproxy...

First of all I was trying to remove requirement to add jar of appropriate
communication client to agent's classpath. Scheme with delegating communication
client was used and client is loaded by remote classloader and passed to the
security proxy now.

Second to fully eliminate requirement to add some jars to the agent's classpath
I've incorporated DelegatingCommClient, SecurityProxyCommClient and
CommunicationClient to the agents' jars. I think this change is reasonable in
the light of the fact that agents will use communication clients for all
communications with JavaTest very soon.

Thanks,
Alexander

---------------------------------------------------------------------
To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
For additional commands, e-mail: meframework-help@cqme.dev.java.net

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
Vladimir Sizikov

Alexander,

The changes look good overall.

Some comments:

1. I'm returning your a favor you just did for Dmitry, when asked to
remove some commented out code... :) In SecurityProxyCommClient.java,
there is a big chunk of code commented out now. Should be deleted.

2. Please add javadoc comments to all new public APIs, like:
DelegatingCommClient,SecurityProxyCommClient.

3. I think that with introduction of DelegatingCommClient, the javadoc
comments for SecurityProxyCommClient should change, shouldn't
they?

Also, please double check with Alexey Y. that these changes indeed resolve
the reported regression.

Thanks,
--Vladimir

On Fri, Apr 27, 2007 at 03:33:13PM +0400, Alexander Alexeev wrote:
> Hi Vladimir,
>
> please review fix for [Issue 104]: "REGRESSION: some original RI's startup
> scripts don't work with current version of ME Framework". The changesets:
> http://fisheye4.cenqua.com/changelog/cqme/branches/users/skavas/secproxy...
> http://fisheye4.cenqua.com/changelog/cqme/branches/users/skavas/secproxy...
>
> First of all I was trying to remove requirement to add jar of appropriate
> communication client to agent's classpath. Scheme with delegating
> communication
> client was used and client is loaded by remote classloader and passed to the
> security proxy now.
>
> Second to fully eliminate requirement to add some jars to the agent's
> classpath
> I've incorporated DelegatingCommClient, SecurityProxyCommClient and
> CommunicationClient to the agents' jars. I think this change is reasonable
> in
> the light of the fact that agents will use communication clients for all
> communications with JavaTest very soon.
>
> Thanks,
> Alexander

---------------------------------------------------------------------
To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
For additional commands, e-mail: meframework-help@cqme.dev.java.net

Alexander Alexeev

Hi Vladimir,

please see the changes on:
http://fisheye4.cenqua.com/changelog/cqme/?cs=785

Thanks,
Alexander

Vladimir Sizikov wrote:
> Alexander,
>
> The changes look good overall.
>
> Some comments:
>
> 1. I'm returning your a favor you just did for Dmitry, when asked to
> remove some commented out code... :) In SecurityProxyCommClient.java,
> there is a big chunk of code commented out now. Should be deleted.
>
> 2. Please add javadoc comments to all new public APIs, like:
> DelegatingCommClient,SecurityProxyCommClient.
>
> 3. I think that with introduction of DelegatingCommClient, the javadoc
> comments for SecurityProxyCommClient should change, shouldn't
> they?
>
> Also, please double check with Alexey Y. that these changes indeed resolve
> the reported regression.
>
> Thanks,
> --Vladimir
>
> On Fri, Apr 27, 2007 at 03:33:13PM +0400, Alexander Alexeev wrote:
>> Hi Vladimir,
>>
>> please review fix for [Issue 104]: "REGRESSION: some original RI's startup
>> scripts don't work with current version of ME Framework". The changesets:
>> http://fisheye4.cenqua.com/changelog/cqme/branches/users/skavas/secproxy...
>> http://fisheye4.cenqua.com/changelog/cqme/branches/users/skavas/secproxy...
>>
>> First of all I was trying to remove requirement to add jar of appropriate
>> communication client to agent's classpath. Scheme with delegating
>> communication
>> client was used and client is loaded by remote classloader and passed to the
>> security proxy now.
>>
>> Second to fully eliminate requirement to add some jars to the agent's
>> classpath
>> I've incorporated DelegatingCommClient, SecurityProxyCommClient and
>> CommunicationClient to the agents' jars. I think this change is reasonable
>> in
>> the light of the fact that agents will use communication clients for all
>> communications with JavaTest very soon.
>>
>> Thanks,
>> Alexander
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
> For additional commands, e-mail: meframework-help@cqme.dev.java.net
>

---------------------------------------------------------------------
To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
For additional commands, e-mail: meframework-help@cqme.dev.java.net

Vladimir Sizikov

Alexander,

Looks good. Just add 'dots' at the end of the sentences in javadocs,
and commit. :)

Thanks,
--Vladimir

On Sat, Apr 28, 2007 at 12:18:04AM +0400, Alexander Alexeev wrote:
> Hi Vladimir,
>
> please see the changes on:
> http://fisheye4.cenqua.com/changelog/cqme/?cs=785
>
> Thanks,
> Alexander
>
> Vladimir Sizikov wrote:
> >Alexander,
> >
> >The changes look good overall.
> >
> >Some comments:
> >
> >1. I'm returning your a favor you just did for Dmitry, when asked to
> > remove some commented out code... :) In SecurityProxyCommClient.java,
> > there is a big chunk of code commented out now. Should be deleted.
> >
> >2. Please add javadoc comments to all new public APIs, like:
> > DelegatingCommClient,SecurityProxyCommClient.
> >
> >3. I think that with introduction of DelegatingCommClient, the javadoc
> > comments for SecurityProxyCommClient should change, shouldn't
> > they?
> >
> >Also, please double check with Alexey Y. that these changes indeed resolve
> >the reported regression.
> >
> >Thanks,
> > --Vladimir
> >
> >On Fri, Apr 27, 2007 at 03:33:13PM +0400, Alexander Alexeev wrote:
> >>Hi Vladimir,
> >>
> >>please review fix for [Issue 104]: "REGRESSION: some original RI's
> >>startup scripts don't work with current version of ME Framework". The
> >>changesets:
> >>http://fisheye4.cenqua.com/changelog/cqme/branches/users/skavas/secproxy?cs=748
> >>http://fisheye4.cenqua.com/changelog/cqme/branches/users/skavas/secproxy?cs=763
> >>
> >>First of all I was trying to remove requirement to add jar of appropriate
> >>communication client to agent's classpath. Scheme with delegating
> >>communication
> >>client was used and client is loaded by remote classloader and passed to
> >>the
> >>security proxy now.
> >>
> >>Second to fully eliminate requirement to add some jars to the agent's
> >>classpath
> >>I've incorporated DelegatingCommClient, SecurityProxyCommClient and
> >>CommunicationClient to the agents' jars. I think this change is
> >>reasonable in
> >>the light of the fact that agents will use communication clients for all
> >>communications with JavaTest very soon.
> >>
> >>Thanks,
> >>Alexander
> >
> >---------------------------------------------------------------------
> >To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
> >For additional commands, e-mail: meframework-help@cqme.dev.java.net
> >
>

---------------------------------------------------------------------
To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
For additional commands, e-mail: meframework-help@cqme.dev.java.net