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




ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
 (lines 364 - 367)
<https://reviews.apache.org/r/48494/#comment202923>

    Since there is no computational expensive calculation in this block, the if 
statement can be omitted. isDebugEnabled is used when logging the message needs 
some calculation that is used by the debug logging only.



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DirectoriesTrimmingStrategy.java
 (line 25)
<https://reviews.apache.org/r/48494/#comment202927>

    Is it possible that an element in the comma separated string contains space?
    
    If it is so, then maybe this whole function could be replaced by:
    return stringToTrim.replaceAll("\\s*,+\\s*", ",");
    (replacing sequences containing at least one comma and spaces around them 
to a single comma. It also removes trailing and leading commas, but preserves 
spaces within the elements. You can try it on http://regex101.com)



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DirectoriesTrimmingStrategy.java
 (lines 26 - 33)
<https://reviews.apache.org/r/48494/#comment202926>

    If the regex in my previuos comment cannot be used, then you can use 
StringUtils to simplify this part:
    return StringUtils.join(stringsArray, ",");



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PropertyValueTrimmingStrategyDefiner.java
 (line 41)
<https://reviews.apache.org/r/48494/#comment202928>

    Simplify this by swapping the constant and the variable (you can use it as 
a thumbrule)
    if("directory".equals(type) || "directories".equals(type))



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PropertyValueTrimmingStrategyDefiner.java
 (line 43)
<https://reviews.apache.org/r/48494/#comment202929>

    Swap constant and variable


- Daniel Gergely


On jún. 14, 2016, 1:45 du, Dmytro Sen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48494/
> -----------------------------------------------------------
> 
> (Updated jún. 14, 2016, 1:45 du)
> 
> 
> Review request for Ambari, Robert Nettleton, Sebastian Toader, and Vitalyi 
> Brodetskyi.
> 
> 
> Bugs: AMBARI-17146
>     https://issues.apache.org/jira/browse/AMBARI-17146
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Implement config values trimming for deployment via blueprint as we do in UI
> 
>   trimProperty: function (property) {
>     var displayType = Em.get(property, 'displayType');
>     var value = Em.get(property, 'value');
>     var name = Em.get(property, 'name');
>     var rez;
>     switch (displayType) {
>       case 'directories':
>       case 'directory':
>         rez = value.replace(/,/g, ' ').trim().split(/\s+/g).join(',');
>         break;
>       case 'host':
>         rez = value.trim();
>         break;
>       case 'password':
>         break;
>       default:
>         if (name == 'javax.jdo.option.ConnectionURL' || name == 
> 'oozie.service.JPAService.jdbc.url') {
>           rez = value.trim();
>         }
>         rez = (typeof value == 'string') ? value.replace(/(\s+$)/g, '') : 
> value;
>     }
>     return ((rez == '') || (rez == undefined)) ? value : rez;
>   },
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  9094698 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultTrimmingStrategy.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DeleteSpacesAtTheEndTrimmingStrategy.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DirectoriesTrimmingStrategy.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PasswordTrimmingStrategy.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PropertyValueTrimmingStrategyDefiner.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java
>  ad8d4f9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/TrimmingStrategy.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  cda8fb8 
> 
> Diff: https://reviews.apache.org/r/48494/diff/
> 
> 
> Testing
> -------
> 
> Unit tests and manual tests passed
> 
> 
> Thanks,
> 
> Dmytro Sen
> 
>

Reply via email to