> On May 10, 2016, 8:22 a.m., Puneet Gupta wrote: > > lens-client/src/main/java/org/apache/lens/client/LensConnection.java, line > > 214 > > <https://reviews.apache.org/r/47121/diff/2/?file=1376577#file1376577line214> > > > > should we have only one public close*() method ? Two methods can be > > confusing for end user. > > If we need to return APIResult, we can not adhere to Closeable Interface
So there are two approaches, each with trade-offs 1. Change the return type to void. This makes sense, since close should just be closing everything without the client needing to take care of the return output. This is what I did in revision 1. The down-side is that existing code that's using this method and depending on return type will become incompatible. 2. Have two close method, as done in revision 2. More confusing, still makes existing clients to change. I'm also inclined towards approach 1, let me know your thoughts. - Rajat ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47121/#review132365 ----------------------------------------------------------- On May 9, 2016, 8:48 p.m., Rajat Khandelwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47121/ > ----------------------------------------------------------- > > (Updated May 9, 2016, 8:48 p.m.) > > > Review request for lens. > > > Bugs: LENS-1010 > https://issues.apache.org/jira/browse/LENS-1010 > > > Repository: lens > > > Description > ------- > > This would allow users to use lens client in try-with-resources clause > without worrying about closing manually. > > https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html > > > Diffs > ----- > > lens-client/src/main/java/org/apache/lens/client/LensClient.java > 43da691aff52bc108f99033fac2e8e068ae1c216 > lens-client/src/main/java/org/apache/lens/client/LensConnection.java > 49518662eb17c1fd24deb26ddd46a36c8826bc5b > > Diff: https://reviews.apache.org/r/47121/diff/ > > > Testing > ------- > > > Thanks, > > Rajat Khandelwal > >
