> On 三月 23, 2017, 11:35 a.m., András Piros wrote:
> > core/src/main/java/org/apache/oozie/servlet/V0JobServlet.java
> > Lines 280-285 (patched)
> > <https://reviews.apache.org/r/57870/diff/1/?file=1672529#file1672529line280>
> >
> >     Why not have this default implementation in `BaseJobServlet` instead?
> 
> Alonzo Zhou wrote:
>     hi, i noticed that the methods in BaseJobServlet was defined as an 
> abstract method in addition to doGet and doPut. in order to maintain 
> consistency, then i defined getWfActionsInCoord as an abstract.
> 
> András Piros wrote:
>     There are already non-abstract methods like `checkAuthorizationForApp()` 
> present in `BaseJobServlet`, so I wouldn't mind putting there a `protected` 
> method those implementation is the same for all children. Having duplicate 
> code is never OK - [DRY 
> principle](https://en.wikipedia.org/wiki/Don't_repeat_yourself).

Thanks for your advice, i'll fix it right away.  and I will remember that 
"Having duplicate code is never OK".


- Alonzo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57870/#review169856
-----------------------------------------------------------


On 三月 28, 2017, 9:36 a.m., Alonzo Zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57870/
> -----------------------------------------------------------
> 
> (Updated 三月 28, 2017, 9:36 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> More detailed view of the coordinator’s history can be observed from 
> perspective of  workflow action.
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/client/rest/RestConstants.java 
> 8ddb1f8 
>   core/src/main/java/org/apache/oozie/CoordinatorEngine.java 2f9f822 
>   core/src/main/java/org/apache/oozie/ErrorCode.java b03ad06 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordWfActionInfoXCommand.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/servlet/BaseJobServlet.java 03acbc1 
>   core/src/main/java/org/apache/oozie/servlet/V0JobServlet.java d3b4689 
>   core/src/main/java/org/apache/oozie/servlet/V1JobServlet.java 9356768 
>   core/src/main/java/org/apache/oozie/servlet/V2JobServlet.java 6c30f5d 
>   core/src/test/java/org/apache/oozie/client/TestOozieCLI.java 564db2a 
>   
> core/src/test/java/org/apache/oozie/command/coord/TestCoordWfActionInfoXCommand.java
>  PRE-CREATION 
>   
> core/src/test/java/org/apache/oozie/servlet/MockCoordinatorEngineService.java 
> 4fc8653 
>   core/src/test/java/org/apache/oozie/servlet/TestV2JobServlet.java fb203a6 
> 
> 
> Diff: https://reviews.apache.org/r/57870/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alonzo Zhou
> 
>

Reply via email to