geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gianny Damour (JIRA)" <...@geronimo.apache.org>
Subject [jira] Commented: (GERONIMO-2135) Improve the ActiveMQ GBeans
Date Thu, 22 Jun 2006 03:17:30 GMT
    [ http://issues.apache.org/jira/browse/GERONIMO-2135?page=comments#action_12417237 ] 

Gianny Damour commented on GERONIMO-2135:
-----------------------------------------

I had a look to the patch and vote +1 for it. Some more details:
1. is fixed.
2. is not fixed.
3. is partially fixed
4. is not fixed
5., 6. and 7. do not know
8. is fixed. Also, it seems that we also avoid "abstract" from methods in interfaces (one
occurence in BrokerServiceGBean).


> Improve the ActiveMQ GBeans
> ---------------------------
>
>          Key: GERONIMO-2135
>          URL: http://issues.apache.org/jira/browse/GERONIMO-2135
>      Project: Geronimo
>         Type: Improvement
>     Security: public(Regular issues) 
>   Components: ActiveMQ
>     Reporter: Hiram Chirino
>     Assignee: Hiram Chirino
>      Fix For: 1.2
>  Attachments: GERONIMO-2135.patch
>
> Suggestions by David Jencks:
> I think that this gbean adaptation code should be in geronimo rather
> than amq.  I'm OK with applying it as is but would prefer some issues
> to be addressed first or, even better,  immediately after the
> transfer (assuming it is done with svn mv).
> 1. DataSourceReference should be replaced by the geronimo class that
> does the same thing, ConnectionFactorySource.
> 2. I think it would be preferable to get the module/configuration
> classloader in the constructor as a magic attribute and use it in
> BrokerServiceGBeanImpl.doStart rather than the classloader of
> BrokerServiceGBeanImpl.
> 3. Same for TransportConnectorGBeanImpl.
> 4. This is a question, not really an issue, about this code:
> +    protected TransportConnector createBrokerConnector(String url)
> throws Exception {
> +        return brokerService.getBrokerContainer().addConnector(url);
> +    }
> To me it seems like this code is combining the functions of factory
> object and container.  Is this necessary and appropriate?  I'd be
> more comfortable with
> Connector connector = ConnectorFactory.createConnector(url);
> brokerService.getBrokerContainer().addConnector(connector);
> I find that the combination style typically creates problems whenever
> trying to extend stuff, say by wrapping the connector.  What do you
> think?
> 5. hardcoding the protocols in ActiveMQManagerGBean seems like a
> temporary expedient at best.
> 6. javadoc on public JMSConnector addConnector( ... in the manager
> gbean seems wrong... does not appear to return an object name.
> 7. Typo and innaccuracies in the first package.html... this stuff is
> only going to work in geronimo, jsr77/88 is not enough.
> 8. I'm not sure exactly what our official policy is but I prefer to
> remove "public" from methods in interfaces since it is the only
> choice and implied.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


Mime
View raw message