> On Nov. 11, 2014, 2:27 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java, > > line 315 > > <https://reviews.apache.org/r/27700/diff/2/?file=754414#file754414line315> > > > > Use braces for all if statements. Not sure if this is explicitly > > called out in our coding standards but not using braces is generally > > frowned upon as it is error prone.
All `if` (and `else`) blocks use braces as of [revision 3](https://reviews.apache.org/r/27700/diff/3/) > On Nov. 11, 2014, 2:27 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java, > > line 114 > > <https://reviews.apache.org/r/27700/diff/2/?file=754417#file754417line114> > > > > javadoc > > > > Seems odd that only the setters are in the interface Added getters and javadocs to the interface as of in [revision 3](https://reviews.apache.org/r/27700/diff/3/) > On Nov. 11, 2014, 2:27 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java, > > line 123 > > <https://reviews.apache.org/r/27700/diff/2/?file=754417#file754417line123> > > > > javadoc Added javadocs as of [revision 3](https://reviews.apache.org/r/27700/diff/3/) > On Nov. 11, 2014, 2:27 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java, > > line 37 > > <https://reviews.apache.org/r/27700/diff/2/?file=754419#file754419line37> > > > > typo > > is -> it's Fixed appropriately as of [revision 3](https://reviews.apache.org/r/27700/diff/3/) > On Nov. 11, 2014, 2:27 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java, > > line 59 > > <https://reviews.apache.org/r/27700/diff/2/?file=754419#file754419line59> > > > > javadoc for members Added javadocs as of [revision 3](https://reviews.apache.org/r/27700/diff/3/) > On Nov. 11, 2014, 2:27 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java, > > line 228 > > <https://reviews.apache.org/r/27700/diff/2/?file=754419#file754419line228> > > > > if join() is interrupted, do we want to propagate the interrupt to the > > worker thread? Propagated interrupt as suggested in [revision 3](https://reviews.apache.org/r/27700/diff/3/) > On Nov. 11, 2014, 2:27 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostServerActionEvent.java, > > line 24 > > <https://reviews.apache.org/r/27700/diff/2/?file=754424#file754424line24> > > > > class is missing all javadoc's Added javadocs in [revision 3](https://reviews.apache.org/r/27700/diff/3/) - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27700/#review60796 ----------------------------------------------------------- On Nov. 11, 2014, 8:43 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27700/ > ----------------------------------------------------------- > > (Updated Nov. 11, 2014, 8:43 p.m.) > > > Review request for Ambari, dilli dorai, Jonathan Hurley, John Speidel, Nate > Cole, Robert Nettleton, and Sid Wagle. > > > Bugs: AMBARI-7985 > https://issues.apache.org/jira/browse/AMBARI-7985 > > > Repository: ambari > > > Description > ------- > > Ambari currently handles client-/agent-side commands; however there is no > ability to handle server-side commands. Server-side commands should be > specified as a task in a stage and managed along with the stage. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessor.java > 1f99b4a > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java > 5e879cc > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionManager.java > e2fad5f > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionScheduler.java > 81fee75 > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java > bbc5ac3 > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > d74510a > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java > 6920a9e > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerAction.java > be885b5 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionManager.java > 011cf06 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionManagerImpl.java > 3a16c77 > > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentHostEvent.java > 78590fc > > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentHostEventType.java > b43ac9c > > ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostServerActionEvent.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostUpgradeEvent.java > 8b375fe > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java > 36acbc2 > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionManager.java > 5a2c467 > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java > 7224924 > > ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java > 6e78b1d > > ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java > e1e25e0 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/MockServerAction.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/ServerActionExecutorTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/27700/diff/ > > > Testing > ------- > > ------------------------------------------------------------------------------- > Test set: org.apache.ambari.server.serveraction.ServerActionExecutorTest > ------------------------------------------------------------------------------- > Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 41.822 sec > > > Thanks, > > Robert Levas > >
