> On Dec. 18, 2012, 2:36 p.m., Alejandro Abdelnur wrote: > > patch looks good except for the 2 comments on 'condition inversion' > > changes. > > > > 1* Those changes are not related to this patch > > 2* The current behavior is correct > > 3* With the current IF logic the method has a single exit point, the end of > > the method, that is preferable. > > > > Ashish Vaidya wrote: > I was trying to reduce the code indentation caused by such IF conditions, > making the code more readable.
A rule we try to follow, except when the resulting code is overly complex, is that a method as a single exit point, the end of the method. - Alejandro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8661/#review14647 ----------------------------------------------------------- On Dec. 18, 2012, 2:23 a.m., Ashish Vaidya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8661/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2012, 2:23 a.m.) > > > Review request for oozie, Mohammad Islam, Alejandro Abdelnur, Mona Chitnis, > Virag Kothari, and Rohini Palaniswamy. > > > Description > ------- > > In JavaActionExecutor.java addShareLib() should use actionLibPath instead of > appPath so that IllegalArgumentException is not thrown in a federated setup. > > > This addresses bug OOZIE-1137. > https://issues.apache.org/jira/browse/OOZIE-1137 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java > 1423065 > > Diff: https://reviews.apache.org/r/8661/diff/ > > > Testing > ------- > > > Thanks, > > Ashish Vaidya > >
