Return-Path: X-Original-To: apmail-falcon-dev-archive@minotaur.apache.org Delivered-To: apmail-falcon-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 9CA5718FE2 for ; Mon, 18 Jan 2016 11:05:11 +0000 (UTC) Received: (qmail 90106 invoked by uid 500); 18 Jan 2016 11:05:06 -0000 Delivered-To: apmail-falcon-dev-archive@falcon.apache.org Received: (qmail 90062 invoked by uid 500); 18 Jan 2016 11:05:06 -0000 Mailing-List: contact dev-help@falcon.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@falcon.apache.org Delivered-To: mailing list dev@falcon.apache.org Received: (qmail 90051 invoked by uid 99); 18 Jan 2016 11:05:06 -0000 Received: from Unknown (HELO spamd3-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 18 Jan 2016 11:05:06 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd3-us-west.apache.org (ASF Mail Server at spamd3-us-west.apache.org) with ESMTP id C406F18065A for ; Mon, 18 Jan 2016 11:05:05 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd3-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 4.628 X-Spam-Level: **** X-Spam-Status: No, score=4.628 tagged_above=-999 required=6.31 tests=[DKIM_ADSP_CUSTOM_MED=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.001, HTML_MESSAGE=3, KAM_LAZY_DOMAIN_SECURITY=1, NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RP_MATCHES_RCVD=-0.554] autolearn=disabled Received: from mx1-us-east.apache.org ([10.40.0.8]) by localhost (spamd3-us-west.apache.org [10.40.0.10]) (amavisd-new, port 10024) with ESMTP id DXUWJhb_Siky for ; Mon, 18 Jan 2016 11:05:04 +0000 (UTC) Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx1-us-east.apache.org (ASF Mail Server at mx1-us-east.apache.org) with SMTP id 1D02442ACA for ; Mon, 18 Jan 2016 11:05:04 +0000 (UTC) Received: (qmail 90039 invoked by uid 99); 18 Jan 2016 11:05:03 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 18 Jan 2016 11:05:03 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id C7588281B09; Mon, 18 Jan 2016 11:05:02 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============5577433745136575695==" MIME-Version: 1.0 Subject: Re: Review Request 42449: Code Refactoring for Falcon Client From: "Peeyush Bishnoi" To: "Peeyush Bishnoi" , "Falcon" , "Ajay Yadava" Date: Mon, 18 Jan 2016 11:05:02 -0000 Message-ID: <20160118110502.32038.6281@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: "Peeyush Bishnoi" X-ReviewGroup: Falcon X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/42449/ X-Sender: "Peeyush Bishnoi" References: <20160118072417.32038.29547@reviews.apache.org> In-Reply-To: <20160118072417.32038.29547@reviews.apache.org> Reply-To: "Peeyush Bishnoi" X-ReviewRequest-Repository: falcon-git --===============5577433745136575695== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42449/#review114981 ----------------------------------------------------------- client/src/main/java/org/apache/falcon/client/FalconClient.java (line 93) Many of these parameters has already already defined in FalconCLI class. Instead of defining these parameters again in FalconClient, should not we put all the common parameters in one class and get used by both the classes. client/src/main/java/org/apache/falcon/client/FalconClient.java (line 728) In the given function you don't have a check , if any passed params is null or not. client/src/main/java/org/apache/falcon/client/FalconClient.java (line 730) Should not be the WebResource resource initialized before for() {} loop and then the parameters should be assigned to resource to perform ResourceBuilder. client/src/main/java/org/apache/falcon/client/FalconClient.java (line 747) Shall we use StringUtils.isNotBlank(). - Peeyush Bishnoi On Jan. 18, 2016, 7:24 a.m., Ajay Yadava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42449/ > ----------------------------------------------------------- > > (Updated Jan. 18, 2016, 7:24 a.m.) > > > Review request for Falcon. > > > Bugs: FALCON-1707 > https://issues.apache.org/jira/browse/FALCON-1707 > > > Repository: falcon-git > > > Description > ------- > > Code Refactoring for Falcon Client > > > Diffs > ----- > > client/src/main/java/org/apache/falcon/client/FalconClient.java 3f3a871 > > Diff: https://reviews.apache.org/r/42449/diff/ > > > Testing > ------- > > > Thanks, > > Ajay Yadava > > --===============5577433745136575695==--