geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "John Sisson (JIRA)" <>
Subject [jira] Commented: (GERONIMO-2135) Improve the ActiveMQ GBeans
Date Fri, 30 Jun 2006 01:04:30 GMT
    [ ] 

John Sisson commented on GERONIMO-2135:

Comment from djencks on dev list  22nd June (

This patch addresses my major concerns. I consider it
a bug fix and thus not requiring further voting beyond
my previous +1, but in case anyone disagrees, I've
applied it and tested it to the best of my abilities
and vote +1.

david jencks

> Improve the ActiveMQ GBeans
> ---------------------------
>          Key: GERONIMO-2135
>          URL:
>      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:
For more information on JIRA, see:

View raw message