ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Hurley" <jhur...@hortonworks.com>
Subject Re: Review Request 28624: Views: Add new view parameter fields.
Date Wed, 03 Dec 2014 03:45:06 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28624/#review63641
-----------------------------------------------------------

Ship it!


Looks good; a minor nit about @Column vs @Basic. I dislike @Basic :)


ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewParameterEntity.java
<https://reviews.apache.org/r/28624/#comment105915>

    In this case, you don't need both @Basic and @Column. @Basic is really only used to override
the fetch type; we use @Column usually with an explicit name. I like using an explicit name
since you change the field without repurcussions.


- Jonathan Hurley


On Dec. 2, 2014, 9:33 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28624/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2014, 9:33 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.
> 
> 
> Bugs: AMBARI-8519
>     https://issues.apache.org/jira/browse/AMBARI-8519
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Add the following view parameter fields :
> 
> # <label> (a user friendly display in the UI). This element is optional. Admin
View: If label is not specified, use the name.
> # <placeholder> text (used as placeholder text in the UI inputfield). This element
is optional. Admin View: If not specified, blank. 
> # <default> value. Admin View: if set, use as the default value and shown in the
inputfield. This element is optional. This will show instead of placeholder text (if placeholder
is specified).
> 
> For example ...
> 
> {code}
>     <parameter>
>         <name>ambari.server.username</name>
>         <description>Ambari administrator username (for example: admin)</description>
> 
>         <label>UserName</label>
>         <placeholder>some placeholder text...</placeholder>
>         <default>admin</default>
> 
>         <required>true</required>
>     </parameter>
> {code}
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java
a944e95 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java
1dd1b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewParameterEntity.java
0c4c70f 
>   ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog200.java
79e0daa 
>   ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java 77c7c60

>   ambari-server/src/main/java/org/apache/ambari/server/view/configuration/ParameterConfig.java
827b5e7 
>   ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 4bd374d 
>   ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 9eb4e3a 
>   ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql e8f20f6 
>   ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql 059662f 
>   ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 232b283 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProviderTest.java
023cba5 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog200Test.java
c920905 
>   ambari-server/src/test/java/org/apache/ambari/server/view/ViewContextImplTest.java
519d010 
>   ambari-server/src/test/java/org/apache/ambari/server/view/configuration/ParameterConfigTest.java
e3350bc 
>   ambari-server/src/test/java/org/apache/ambari/server/view/configuration/ViewConfigTest.java
04cbe2b 
>   ambari-server/src/test/resources/test_view.xml 5742bfe 
>   ambari-views/src/main/resources/view.xsd a9e5b12 
> 
> Diff: https://reviews.apache.org/r/28624/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added.  Manual testing for default value.
> 
> All existing tests pass ...
> 
> Results :
> 
> Tests run: 2316, Failures: 0, Errors: 0, Skipped: 22
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message