> On April 11, 2016, 1:16 p.m., Alok Lal wrote: > > My vote: -0 > > > > In several of the cases the extra arguments exist either for future > > functionality that someone had in mind or to allow for others to easily > > extend behavior if they so desire, e.g. in case of custom plugins. > > - event passed to DbAuditProvider.preCreate() > > - Removal of vaidationMessage argument from RangerServiceDef’s ctor > > - Removal of fsOwner and superGroup RangerHdfsAuthorizer.isAccessAllowed() > > - Removal of output objects, context, sessionContext and groups from > > RangerHiveAuthorizer.handleDfsCommand(), etc. > > > > One can argue about suitability of having unused arguments in place for > > future functionality. But my guess is that in most cases the arguments > > were added after some deliberation. > > > > While, it is virtuous to try and eliminate warnings I am hesitant about us > > making changes to eliminate UnusedFormalParameter warning. Because of > > above reasons I propose that we should let this exception remain. > > Colm O hEigeartaigh wrote: > Thanks for your review Alok. The counterargument to the points you've > raised is that almost all of the changes are internal - i.e. private methods. > So it is not possible for users to extends these methods anyway. If I were to > limit the changes strictly to private methods would it change your -0? If not > then I will just leave the existing behaviour as it is.
Point taken; private methods don't have that defense. However, I suppose we would also have to police its regression, e.g. by adding exclusion to all non-private methods' use of unused parameters? - Alok ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45679/#review128235 ----------------------------------------------------------- On April 4, 2016, 7:04 a.m., Colm O hEigeartaigh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45679/ > ----------------------------------------------------------- > > (Updated April 4, 2016, 7:04 a.m.) > > > Review request for ranger. > > > Repository: ranger > > > Description > ------- > > Fix PMD Unused Formal Parameters > > > Diffs > ----- > > > agents-audit/src/main/java/org/apache/ranger/audit/provider/DbAuditProvider.java > 8319d36 > > agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceDef.java > 1dac6e8 > dev-support/ranger-pmd-ruleset.xml f82d831 > > hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java > b463da3 > > hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/agent/HadoopAuthClassTransformer.java > ace400b > > hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java > 9c57968 > > plugin-solr/src/main/java/org/apache/ranger/authorization/solr/authorizer/RangerSolrAuthorizer.java > 151f23e > security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java 551ec2e > security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java > 5905fc9 > security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java e9c8394 > security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java > cf66fc1 > security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java > c25b989 > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > 0dbd042 > > Diff: https://reviews.apache.org/r/45679/diff/ > > > Testing > ------- > > > Thanks, > > Colm O hEigeartaigh > >
