----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3505/#review4392 -----------------------------------------------------------
Some nits, but otherwise looks good! Thanks Bruno! flume-ng-core/src/main/java/org/apache/flume/sink/SinkType.java <https://reviews.apache.org/r/3505/#comment9895> nit: trailing space. flume-ng-sinks/flume-jms-sink/src/main/java/org/apache/flume/sink/jms/JMSSink.java <https://reviews.apache.org/r/3505/#comment9886> style nit: no need to qualify logger. flume-ng-sinks/flume-jms-sink/src/main/java/org/apache/flume/sink/jms/JMSSink.java <https://reviews.apache.org/r/3505/#comment9887> nit: Rogue tab character. flume-ng-sinks/flume-jms-sink/src/main/java/org/apache/flume/sink/jms/JMSSink.java <https://reviews.apache.org/r/3505/#comment9888> nit: tab. flume-ng-sinks/flume-jms-sink/src/main/java/org/apache/flume/sink/jms/JMSSink.java <https://reviews.apache.org/r/3505/#comment9890> style: indent. flume-ng-sinks/flume-jms-sink/src/main/java/org/apache/flume/sink/jms/JMSSink.java <https://reviews.apache.org/r/3505/#comment9889> style: indent. flume-ng-sinks/flume-jms-sink/src/main/java/org/apache/flume/sink/jms/JMSSink.java <https://reviews.apache.org/r/3505/#comment9891> nit: tab. flume-ng-sinks/flume-jms-sink/src/main/java/org/apache/flume/sink/jms/JMSSink.java <https://reviews.apache.org/r/3505/#comment9892> nit: tab. flume-ng-sinks/flume-jms-sink/src/main/java/org/apache/flume/sink/jms/JMSSink.java <https://reviews.apache.org/r/3505/#comment9893> nit: tab. flume-ng-sinks/flume-jms-sink/src/test/java/org/apache/flume/sink/jms/TestJMSSink.java <https://reviews.apache.org/r/3505/#comment9894> nit: double newline and space. pom.xml <https://reviews.apache.org/r/3505/#comment9896> I wonder what we should do about plugins going forward. Obviously each plugin has its own deps but we don't necessarily want people to have to deal with a lot of bloat for features they'll never use. This is fine for now, but we should start a FEP or something to figure it out. - Eric On 2012-01-14 05:47:18, Bruno Mahé wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3505/ > ----------------------------------------------------------- > > (Updated 2012-01-14 05:47:18) > > > Review request for Flume. > > > Summary > ------- > > Add an implementation for a JMS sink > > > This addresses bug FLUME-923. > https://issues.apache.org/jira/browse/FLUME-923 > > > Diffs > ----- > > flume-ng-core/src/main/java/org/apache/flume/sink/SinkType.java 5be602a > flume-ng-dist/pom.xml 5c20252 > flume-ng-node/pom.xml c0e295c > flume-ng-sinks/flume-jms-sink/pom.xml PRE-CREATION > > flume-ng-sinks/flume-jms-sink/src/main/java/org/apache/flume/sink/jms/JMSSink.java > PRE-CREATION > > flume-ng-sinks/flume-jms-sink/src/test/java/org/apache/flume/sink/jms/TestJMSSink.java > PRE-CREATION > flume-ng-sinks/pom.xml 044bf8e > pom.xml 6002358 > > Diff: https://reviews.apache.org/r/3505/diff > > > Testing > ------- > > * Checked unit tests pass, including the one from the JMS sink > * Brought up my own external JMS broker and hooked up a JMS sink on a flume > node > > > Thanks, > > Bruno > >
