> On Feb. 2, 2016, 6:26 a.m., Lenni Kuff wrote: > > code looks fine, with one comment. Seems like we shouldn't be building our > > own option parser though and instead should just use something like apache > > commons-cli
Thanks for the review, Lenni. We are using commons-cli. Commons-cli uses a HashMap under the covers for storing the options groups, which is why you can't depend on the order. - Gregory ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43026/#review117364 ----------------------------------------------------------- On Jan. 31, 2016, 3:31 a.m., Gregory Chanan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43026/ > ----------------------------------------------------------- > > (Updated Jan. 31, 2016, 3:31 a.m.) > > > Review request for sentry, Colin Ma and Sravya Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > SENTRY-749 and SENTRY-995 add tests that check for group options in a certain > order (e.g. [-lp List privilege, -rpr Revoke privilege from role, -lr List > role, -arg Add group to role, -drg Delete group from role, -gpr Grant > privilege to role, -dr Drop role, -cr Create role]), but the order is not > well defined because the org.apache.commons.cli.OptionGroup that is used for > the options stores them in a HashMap. > > So, instead of checking a defined order, we check that all the expected > options exist. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java > 354cf357dd3f74696de4d3eb49d707e980a1a641 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentryShellHive.java > 3907200d7da33faa038d63593a889f05303f7c18 > > Diff: https://reviews.apache.org/r/43026/diff/ > > > Testing > ------- > > Ran the unit tests on multiple machines / JVMs. > > > Thanks, > > Gregory Chanan > >
