Hi Raul! Thank you very much for addressing my comments!
I like the code however there are still a couple of points (I committed everything to ignite-1747): 1. Please take a look at https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-StringOutput 2. org.apache.ignite.stream.mqtt.MqttStreamer#connected - volatile write then read of most likely the same value - I changed it to return true literal. connected = true; return connected; 3. blockUntilConnected - I would name this property syncConn 4. Abbreviation rules violations - connected - conn, executor - exec, values - vals, count - cnt, message - msg 5. What is the point of "connected" member? Client has "isConnected()"? Is it similar to what you want to achieve? 6. Race on stop - 1 thread calls stop and successfully exits the method, retrier is not stopped (awaitTermination has not been called - should it be?) and can connect client back - is this the case? 7. I think we use {@code } instead of <tt>... On more point community should agree on - imports ordering and grouping. I will post another email. --Yakov 2015-10-20 20:39 GMT+03:00 Raul Kripalani <ra...@apache.org>: > Hi Yakov, > > I've had a few free cycles to fix the code style and Javadoc issues you > reported in the MQTT Streamer and I've pushed my changes to branch > ignite-1747. > > Would you mind having a look to see if it adapts better now? > > Thanks, > > *Raúl Kripalani* > PMC & Committer @ Apache Ignite, Apache Camel | Integration, Big Data and > Messaging Engineer > http://about.me/raulkripalani | http://www.linkedin.com/in/raulkripalani > http://blog.raulkr.net | twitter: @raulvk >
