> On Jan. 21, 2016, 4:27 a.m., Nahappan Somasundaram wrote: > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AdminSettingResourceProviderTest.java, > > line 1 > > <https://reviews.apache.org/r/42339/diff/1/?file=1204061#file1204061line1> > > > > Some comments on what each of the test methods do will help when > > debugging.
Each method name is self explanatory what the test method is doing like deleteResource/updateResource. > On Jan. 21, 2016, 4:27 a.m., Nahappan Somasundaram wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java, > > line 41 > > <https://reviews.apache.org/r/42339/diff/1/?file=1204050#file1204050line41> > > > > To reduce noise, it is recommended that changes for refactoring, > > cleanup, style changes, etc., which do not affect the feature have a > > separate JIRA as a prep work. I agree with you, if there are changes not related to JIRA, there is lot more noise. If there are lot of clean up across many files, generally I publish first review with clean up changes and then relevant changes so that reviewers can do a diff and look at clean up or actual changes, the way they want. In this case changes were very minimal and filing a jira is too much overhead, I just put it altogether. > On Jan. 21, 2016, 4:27 a.m., Nahappan Somasundaram wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java, > > line 85 > > <https://reviews.apache.org/r/42339/diff/1/?file=1204050#file1204050line85> > > > > Not really a fan of multiple return statements, unless that is the > > coding convention in Ambari. There is no ambari convention in this regard. There are many files which have one or other pattern. I will revert this change. - Ajit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42339/#review115556 ----------------------------------------------------------- On Jan. 21, 2016, 12:04 a.m., Ajit Kumar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42339/ > ----------------------------------------------------------- > > (Updated Jan. 21, 2016, 12:04 a.m.) > > > Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Nate Cole, > Nahappan Somasundaram, Sumit Mohanty, and Sid Wagle. > > > Bugs: AMBARI-14750 > https://issues.apache.org/jira/browse/AMBARI-14750 > > > Repository: ambari > > > Description > ------- > > Provide CRUD support for admin-settings > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java > c7c5d61b70ea38f22861519783733e9b9ebd415d > > ambari-server/src/main/java/org/apache/ambari/server/api/services/AdminSettingService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java > cce3764b36e4d8a4ca0762329c941405b0966fc2 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AdminSettingResourceProvider.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java > 3801cc3da56769b2dfc5ca1ceb22a8aa95eab63f > > ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java > e367afe4e319a8de441d201d619da4f20b48dcdb > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AdminSettingDAO.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AdminSettingEntity.java > PRE-CREATION > ambari-server/src/main/resources/META-INF/persistence.xml > 3979bc0782840284678d5e2d6f5af797d011aa18 > ambari-server/src/main/resources/key_properties.json > 46a6cf9b3f1d171ed8250df787cb671cce426c02 > ambari-server/src/main/resources/properties.json > 4052ad213eeb5da8475c6e80d9b404fada256fa9 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AdminSettingResourceProviderTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AdminSettingDAOTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/orm/entities/AdminSettingEntityTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/42339/diff/ > > > Testing > ------- > > Tested API through curl > unit tests > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Ambari Main ....................................... SUCCESS [9.890s] > [INFO] Apache Ambari Project POM ......................... SUCCESS [0.060s] > [INFO] Ambari Web ........................................ SUCCESS [2:15.144s] > [INFO] Ambari Views ...................................... SUCCESS [3.885s] > [INFO] Ambari Admin View ................................. SUCCESS [11.634s] > [INFO] ambari-metrics .................................... SUCCESS [0.388s] > [INFO] Ambari Metrics Common ............................. SUCCESS [2.729s] > [INFO] Ambari Metrics Hadoop Sink ........................ SUCCESS [5.239s] > [INFO] Ambari Metrics Flume Sink ......................... SUCCESS [2.931s] > [INFO] Ambari Metrics Kafka Sink ......................... SUCCESS [4.818s] > [INFO] Ambari Metrics Storm Sink ......................... SUCCESS [2.567s] > [INFO] Ambari Metrics Collector .......................... SUCCESS [1:00.778s] > [INFO] Ambari Metrics Monitor ............................ SUCCESS [3.318s] > [INFO] Ambari Metrics Assembly ........................... SUCCESS [59.382s] > [INFO] Ambari Server ..................................... SUCCESS > [1:03:30.478s] > [INFO] Ambari Functional Tests ........................... SUCCESS [1:57.459s] > [INFO] Ambari Agent ...................................... SUCCESS [13.973s] > [INFO] Ambari Client ..................................... SUCCESS [0.041s] > [INFO] Ambari Python Client .............................. SUCCESS [2.306s] > [INFO] Ambari Groovy Client .............................. SUCCESS [12.051s] > [INFO] Ambari Shell ...................................... SUCCESS [0.033s] > [INFO] Ambari Python Shell ............................... SUCCESS [0.863s] > [INFO] Ambari Groovy Shell ............................... SUCCESS [9.254s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 1:11:12.017s > [INFO] Finished at: Wed Jan 20 16:02:44 PST 2016 > [INFO] Final Memory: 223M/1079M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Ajit Kumar > >
