[ http://issues.apache.org/jira/browse/GERONIMO-2135?page=comments#action_12418550 ]
Jason Dillon commented on GERONIMO-2135: ---------------------------------------- +1 *if* m2 pom's are also updated. > 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
