Ravi Nori has posted comments on this change.

Change subject: tools : engine-config is over writing the previous values
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.ovirt.org/#/c/25762/2/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java
File 
backend/manager/tools/src/main/java/org/ovirt/engine/core/config/EngineConfigLogic.java:

Line 404:                 return;
Line 405:             }
Line 406:         }
Line 407:         if (!persist(key, value, version)) {
Line 408:             log.debug("setValue: error concatenating " + key + "'s 
value. No such entry"
> Message should be declared above as other messages with teh relevant place 
Hi Eli,

looks like the log does not have debugFormat method
Line 409:                     + (version == null ? "" : " with version " + 
version) + ".");
Line 410:             throw new IllegalArgumentException("Error setting " + key 
+ "'s value. No such entry"
Line 411:                     + (version == null ? "" : " with version " + 
version) + ".");
Line 412:         }


Line 422: 
Line 423:     private String concatenatedValue(String valueToAppend, String 
currentValue, String delimiter) {
Line 424:         String retValue = currentValue;
Line 425:         if 
(!Arrays.asList(retValue.split(delimiter)).contains(valueToAppend)) {
Line 426:             if (!retValue.endsWith(delimiter)) {
> seems as nested if can be omitted and replace with a simple && condition wi
Hi Eli, The above cant be collapsed. Please take a second look
Line 427:                 retValue += delimiter;
Line 428:             }
Line 429:             retValue += valueToAppend;
Line 430:         }


Line 423:     private String concatenatedValue(String valueToAppend, String 
currentValue, String delimiter) {
Line 424:         String retValue = currentValue;
Line 425:         if 
(!Arrays.asList(retValue.split(delimiter)).contains(valueToAppend)) {
Line 426:             if (!retValue.endsWith(delimiter)) {
Line 427:                 retValue += delimiter;
> Please use StringBuilder here
Will do
Line 428:             }
Line 429:             retValue += valueToAppend;
Line 430:         }
Line 431:         return retValue;


-- 
To view, visit http://gerrit.ovirt.org/25762
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5c70539f47c509e3b8c23b1aa3de41bead36c1b4
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to