> On Dec. 21, 2015, 5:06 a.m., Pallavi Rao wrote: > > client/src/main/java/org/apache/falcon/client/FalconClient.java, line 949 > > <https://reviews.apache.org/r/41587/diff/1/?file=1172720#file1172720line949> > > > > Let us not introduce a new method. I believe you introduced this in > > order to supply FeedInstanceResult to appropriate methods. > > > > It would be better to refactor the code to supply the appropriate class > > in order methods too. > > Ajay Yadava wrote: > I had an offline discussion with Praveen Adlakha. The reason he took this > approach is because that one method is used in several apis. He wants to > tackle all other APIs in a separate JIRA as they will potentially entail > changes on both client side and server side.
Introducing another utility method and updating the code to use this new method (as in case of FalconCLIException) may be acceptable to do in 2 separate JIRAs. But, introduction of a new and duplicate API is confusing and error prone. IMO, any changes to keep a single method must be handled in the same JIRA. For example, if someone has to add a new parameter to getFeedListing, they'll have to do it in 2 places. - Pallavi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41587/#review111437 ----------------------------------------------------------- On Dec. 28, 2015, 2:07 p.m., Praveen Adlakha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41587/ > ----------------------------------------------------------- > > (Updated Dec. 28, 2015, 2:07 p.m.) > > > Review request for Falcon. > > > Bugs: falcon-1565 > https://issues.apache.org/jira/browse/falcon-1565 > > > Repository: falcon-git > > > Description > ------- > > Listing API non-intuitive response if time > endTime > > > Diffs > ----- > > client/src/main/java/org/apache/falcon/cli/FalconInstanceCLI.java 6ea34da > client/src/main/java/org/apache/falcon/client/FalconCLIException.java > 51ef952 > client/src/main/java/org/apache/falcon/client/FalconClient.java aea39a6 > prism/src/main/java/org/apache/falcon/FalconWebException.java 666e3d7 > prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java > da2fea7 > prism/src/main/java/org/apache/falcon/resource/channel/HTTPChannel.java > 6801398 > > prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java > 96c99f0 > > Diff: https://reviews.apache.org/r/41587/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Praveen Adlakha > >
