----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54891/#review159704 -----------------------------------------------------------
Fix it, then Ship it! ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java (lines 40 - 42) <https://reviews.apache.org/r/54891/#comment230748> Could be `final`. ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java (lines 63 - 64) <https://reviews.apache.org/r/54891/#comment230728> Please enclose in {} ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java (lines 68 - 69) <https://reviews.apache.org/r/54891/#comment230729> Please enclose in {} ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java (line 81) <https://reviews.apache.org/r/54891/#comment230731> ServiceComponent is created 2x (line 77, line 81). ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java (lines 101 - 103) <https://reviews.apache.org/r/54891/#comment230736> Could be final. ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java (line 149) <https://reviews.apache.org/r/54891/#comment230754> `Link.getProperties()` is `@Nullable`. ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java (line 150) <https://reviews.apache.org/r/54891/#comment230753> Could be simplified with `CollectionUtils.containsAny`. ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java (line 1) <https://reviews.apache.org/r/54891/#comment230724> Missing license. ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java (line 4) <https://reviews.apache.org/r/54891/#comment230749> Wouldn't `IllegalArgumentException` be OK for the same purpose? ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java (line 31) <https://reviews.apache.org/r/54891/#comment230757> Please move to `com.google` block of imports. ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java (lines 40 - 44) <https://reviews.apache.org/r/54891/#comment230741> Could be `private`. ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java (line 46) <https://reviews.apache.org/r/54891/#comment230756> Could be `final`. ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java (line 57) <https://reviews.apache.org/r/54891/#comment230738> Please do not add `@throws Exception` javadoc, does not add any value. (Also in other test methods and in QuickLinksProfileEvaluatorTest.) ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java (line 77) <https://reviews.apache.org/r/54891/#comment230725> Trailing whitespace. ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java (line 150) <https://reviews.apache.org/r/54891/#comment230755> Unused local variable (also in following two methods). ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java (lines 33 - 37) <https://reviews.apache.org/r/54891/#comment230742> Could be `private`. ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java (line 40) <https://reviews.apache.org/r/54891/#comment230744> Could be `final`. - Attila Doroszlai On Dec. 20, 2016, 1:20 p.m., Balázs Bence Sári wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54891/ > ----------------------------------------------------------- > > (Updated Dec. 20, 2016, 1:20 p.m.) > > > Review request for Ambari, Attila Magyar, Jayush Luniya, Laszlo Puskas, > Sandor Magyari, Sumit Mohanty, and Sebastian Toader. > > > Bugs: AMBARI-19244 > https://issues.apache.org/jira/browse/AMBARI-19244 > > > Repository: ambari > > > Description > ------- > > Profile evaluator is a class that takes a quick links profile as input and > can decide on the visibility of quicklinks based on the received profile. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/Component.java > 7ef0259 > > ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfile.java > 1a1488b > > ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/Service.java > 600872f > > ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/54891/diff/ > > > Testing > ------- > > Wrote new unit tests. Run the ambari-server test suite. Failures are > irrelevant. > > Failed tests: > DataStoreImplTest.testFind:524 expected:<class > DS_DataStoreImplTest$TestEntity_1> but was:<class > DS_DataStoreImplTest$TestSubEntity_1> > DataStoreImplTest.testRemove:473 expected:<class > DS_DataStoreImplTest$TestEntity_1> but was:<class > DS_DataStoreImplTest$TestSubEntity_1> > DataStoreImplTest.testStore_update:355 expected:<class > DS_DataStoreImplTest$TestEntity_1> but was:<class > DS_DataStoreImplTest$TestSubEntity_1> > DataStoreImplTest.testStore_update_longStringValue:424 > Expectation failure on verify: > DynamicEntity.set("DS_id", 99): expected: 1, actual: 0 > Tests in error: > KerberosCheckerTest.testCheckFailed » Unexpected exception, > expected<org.apac... > KerberosCheckerTest.testCheckPassed:62 » ClassCast class > sun.security.provider... > AmbariBasicAuthenticationFilterTest.testOnSuccessfulAuthentication:99 » > Linkage > > Tests run: 4136, Failures: 4, Errors: 3, Skipped: 31 > > > Thanks, > > Balázs Bence Sári > >
