----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29008/#review64971 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java <https://reviews.apache.org/r/29008/#comment107803> Not really our convention, plus you can put the "/" in with {clusterName} ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeChecks.java <https://reviews.apache.org/r/29008/#comment107808> UpgradeCheckProcessor? Typically don't see plurals as class names. ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeChecks.java <https://reviews.apache.org/r/29008/#comment107806> Can we externalize these? ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeChecks.java <https://reviews.apache.org/r/29008/#comment107805> Just inject clusters instead so we can mock this out without needed AmbariServer ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeCheckDto.java <https://reviews.apache.org/r/29008/#comment107804> Can just be called UpgradeCheck. It's not a DTO in the database sense. ambari-server/src/test/java/org/apache/ambari/server/api/resources/ClusterResourceDefinitionTest.java <https://reviews.apache.org/r/29008/#comment107807> This order is correct. Please change your IDE to match: static imports java.* org.* com.* - Nate Cole On Dec. 12, 2014, 3:32 p.m., Yurii Shylov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29008/ > ----------------------------------------------------------- > > (Updated Dec. 12, 2014, 3:32 p.m.) > > > Review request for Ambari, Alejandro Fernandez, Dmitro Lisnichenko, and Nate > Cole. > > > Bugs: AMBARI-8540 > https://issues.apache.org/jira/browse/AMBARI-8540 > > > Repository: ambari > > > Description > ------- > > (see API in attached ticket) > > Only 2 checks are added right now, others will be implemented in next ticket > (as well as some clean-up, there is duplication in current upgrade checks > descriptors) > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java > a353be6 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java > e950d57 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/PreUpgradeCheckService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java > 41bee76 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PreUpgradeCheckResourceProvider.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java > 5c9366a > > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeChecks.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeCheckDto.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeCheckStatus.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeCheckType.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/api/resources/ClusterResourceDefinitionTest.java > 6d897b2 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/ClusterServiceTest.java > 9051059 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/PreUpgradeCheckServiceTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/29008/diff/ > > > Testing > ------- > > Tests are passing > > > Thanks, > > Yurii Shylov > >
