> On Дек. 12, 2014, 8:52 п.п., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java, > > line 458 > > <https://reviews.apache.org/r/29008/diff/1/?file=790681#file790681line458> > > > > Not really our convention, plus you can put the "/" in with > > {clusterName}
Fixed > On Дек. 12, 2014, 8:52 п.п., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeChecks.java, > > line 39 > > <https://reviews.apache.org/r/29008/diff/1/?file=790686#file790686line39> > > > > UpgradeCheckProcessor? Typically don't see plurals as class names. Left it as is for now, probably will rename in next patch. I think we have such convention for classes which encapsulate some work against multiple entities of the same class - Users, Clusters > On Дек. 12, 2014, 8:52 п.п., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeChecks.java, > > line 61 > > <https://reviews.apache.org/r/29008/diff/1/?file=790686#file790686line61> > > > > Can we externalize these? Will do in next patch > On Дек. 12, 2014, 8:52 п.п., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeCheckDto.java, > > lines 27-29 > > <https://reviews.apache.org/r/29008/diff/1/?file=790687#file790687line27> > > > > Can just be called UpgradeCheck. It's not a DTO in the database sense. Fixed > On Дек. 12, 2014, 8:52 п.п., Nate Cole wrote: > > ambari-server/src/test/java/org/apache/ambari/server/api/resources/ClusterResourceDefinitionTest.java, > > lines 21-25 > > <https://reviews.apache.org/r/29008/diff/1/?file=790690#file790690line21> > > > > This order is correct. Please change your IDE to match: > > > > static imports > > java.* > > org.* > > com.* fixed - Yurii ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29008/#review64971 ----------------------------------------------------------- On Дек. 12, 2014, 9:06 п.п., Yurii Shylov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29008/ > ----------------------------------------------------------- > > (Updated Дек. 12, 2014, 9:06 п.п.) > > > 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/UpgradeCheck.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/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 > >
