BITMAG-146: Finished the prototype for the connection with ActiveMQ, and also...

Activity

CR-BITMAG-3 60

Keyboard shortcuts  
  • Summarize the review outcomes (optional)
     
    #permalink

    Details

    Warning: no files are visible, they have all been filtered.
    Participant Role Time Spent Comments Latest Comment
    Author 39m 16 Instead of Log4j libraries, we should be using the LogBac...
    Mikis Seth Sørensen  (deleted user)
    Reviewer completed
    Reviewer - Complete 1h 39m 16 We should consider sticking to only exposing a simple and...
    Reviewer - Complete 2h 21m 28 (2 defects) Missing space
    Total   4h 39m 60 (2 defects)  
    #permalink

    Objectives

    67: BITMAG-146: Finished the prototype for the connection with ActiveMQ, and also created some basic tests for it.

    #permalink

    Summary

    Followup by JOLF.
    Establishment of issue for robustness and the discussion according the the discussed comments in this review.

    Branches in review

    #permalink

    Issues Raised From Comments

    Key Summary State Assignee
    #permalink

    General Comments

    Mikis Seth Sørensen  (deleted user)

    As we discussed in BITMAG-30 Establish Logging, I think we should use the SLF...

    As we discussed in BITMAG-30 Establish Logging, I think we should use the SLF4J, instead of commons logging/log4j. The Dependencies for this are already part of the parent pom. See MessageFactory class for example of usage.

    If we agree on this we could also close BITMAG-30 Establish Logging

    Mikis Seth Sørensen  (deleted user)

    As a general rule, we should run the mvn license:update-project-license befo...

    As a general rule, we should run the

    mvn license:update-project-license
    

    before committing, which will update all file headers with the correct information.

    Perhaps we should consider introducing a precommit script (could be manually run), which is run prior to commiting. This script could also include a run of the regression test.

    Mikis Seth Sørensen  (deleted user)

    There is a lot of ToDo's in the code. We should think of how to handle these ...

    There is a lot of ToDo's in the code. We should think of how to handle these to avoid generating a huge backlog (and somewhat hidden) of outstanding things we need to do.

    Mikis Seth Sørensen  (deleted user)

    We should consider sticking to only exposing a simple and clean interface mes...

    We should consider sticking to only exposing a simple and clean interface message bus interface, as found in MessageBusConnection. I think this should be THE handle to the message system from the protocol module. Perhaps the class should be rename to something more general (connection suggest a network kind of thing).

    All the concrete connection handling code should then be hidden from view behind this class. This includes reconnect/failover etc.

    Mikis Seth Sørensen  (deleted user)

    We should consider sticking to only exposing a simple and clean interface mes...

    We should consider sticking to only exposing a simple and clean interface message bus interface, as found in MessageBusConnection. I think this should be THE handle to the message system from the protocol module. Perhaps the class should be rename to something more general (connection suggest a network kind of thing).

    All the concrete connection handling code should then be hidden from view behind this class. This includes reconnect/failover etc.

    /trunk/.../protocol/ActiveMQConnection.java Changed 13
    Open in IDE #permalink
    /trunk/.../protocol/ConnectionFactory.java Added 10
    Open in IDE #permalink
    /trunk/.../protocol/ConnectionProperty.java Added 16
    Open in IDE #permalink
    /trunk/.../protocol/MessageBusConnection.java Changed 7
    Open in IDE #permalink
    /trunk/.../bitrepository/bitrepositorymessages/ Deleted
    Open in IDE #permalink
    Repository Bitmagasinet does not exist
    /trunk/.../bitrepositorymessages/MessageCreationTest.java Added
    Open in IDE #permalink
    /trunk/.../protocol/ConnectionPropertyTest.java Added 2
    Open in IDE #permalink
    /trunk/.../protocol/MessageBusTest.java Changed 5
    Open in IDE #permalink
    /trunk/bitrepository-protocol/pom.xml Changed 2
    Open in IDE #permalink

    Review updated: Reload | Ignore | Collapse

    You cannot reload the review while writing a comment.

    Log time