You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Andrew Stitcher <as...@apache.org> on 2011/07/15 00:23:53 UTC
Re: Review Request: Recreation of JCA Resource Adapter review
> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java, line 48
> > <https://reviews.apache.org/r/441/diff/3/?file=13133#file13133line48>
> >
> > "QPID-CF" should be external constant
> > r.get() result not checked for null
Agreed - fixed in newer version
> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java, line 127
> > <https://reviews.apache.org/r/441/diff/3/?file=13137#file13137line127>
> >
> > "QPID-CF" should be external constant
Agreed - fixed in newer version
> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java, line 324
> > <https://reviews.apache.org/r/441/diff/3/?file=13140#file13140line324>
> >
> > null check is redundant, since instanceof will return false for a null value of obj
Agreed - fixed in newer version
> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java, line 349
> > <https://reviews.apache.org/r/441/diff/3/?file=13140#file13140line349>
> >
> > hashCode does not meet Object contract with equals and should include clientId in calculation
Agreed - fixed in newer version
> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java, line 355
> > <https://reviews.apache.org/r/441/diff/3/?file=13140#file13140line355>
> >
> > can just use type here?
Agreed - fixed in newer version
> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java, line 357
> > <https://reviews.apache.org/r/441/diff/3/?file=13140#file13140line357>
> >
> > can just use acknowledgeMode here?
Agreed - fixed in newer version
> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java, line 304
> > <https://reviews.apache.org/r/441/diff/3/?file=13146#file13146line304>
> >
> > null check is redundant, due to use of instanceof
Agreed - fixed in newer version
> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java, line 321
> > <https://reviews.apache.org/r/441/diff/3/?file=13146#file13146line321>
> >
> > null check redundant, due to instanceof
Agreed - fixed in newer version
> On 2011-03-03 17:40:21, Andrew Kennedy wrote:
> > /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java, line 176
> > <https://reviews.apache.org/r/441/diff/3/?file=13141#file13141line176>
> >
> > do we really want to use Object's toString here?
On balance we've decided to leave it as is for now, it does no real harm.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/441/#review298
-----------------------------------------------------------
On 2011-02-24 15:12:05, Andrew Stitcher wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/441/
> -----------------------------------------------------------
>
> (Updated 2011-02-24 15:12:05)
>
>
> Review request for qpid.
>
>
> Summary
> -------
>
> Review for a qpid JCA resource adapter.
>
> So far no build infrastructure is included.
>
> I'd also like an opinion as to whether java/jca is the appropriate name for this (I'm thinking perhaps java/ra would be more usual).
>
> Any and all comments welcome.
>
>
> This addresses bug QPID-3044.
> https://issues.apache.org/jira/browse/QPID-3044
>
>
> Diffs
> -----
>
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactory.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRABytesMessage.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryProperties.java PRE-CREATION
> /trunk/qpid/java/build.deps 1070497
> /trunk/qpid/java/build.xml 1070497
> /trunk/qpid/java/jca/build.xml PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/ConnectionFactoryObjectFactory.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionFactoryImpl.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionManager.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionMetaData.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAConnectionRequestInfo.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRACredential.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAException.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRALocalTransaction.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMCFProperties.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnection.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAManagedConnectionFactory.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMapMessage.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessage.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageConsumer.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageListener.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMessageProducer.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAMetaData.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAObjectMessage.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAProperties.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueReceiver.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAQueueSender.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASession.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactory.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRASessionFactoryImpl.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAStreamMessage.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATextMessage.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicPublisher.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRATopicSubscriber.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidResourceAdapter.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/Util.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivation.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidActivationSpec.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/inflow/QpidMessageHandler.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/tm/JBossTransactionManagerLocator.java PRE-CREATION
> /trunk/qpid/java/jca/src/main/resources/META-INF/ra.xml PRE-CREATION
> /trunk/qpid/java/lib/geronimo-j2ee-connector_1.5_spec-2.0.0.jar UNKNOWN
> /trunk/qpid/java/lib/geronimo-jta_1.1_spec-1.1.1.jar UNKNOWN
>
> Diff: https://reviews.apache.org/r/441/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrew
>
>