Allon Mureinik has posted comments on this change.

Change subject: core: Removed unnecessary calls to StringFormat & StringHelper
......................................................................


Patch Set 4: I would prefer that you didn't submit this

(13 inline comments)

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/tags.java
Line 170:         StringBuilder builder = new StringBuilder();
Line 171:         builder.append(StringFormat.format("'%1$s'", gettag_id()));
Line 172: 
Line 173:         for (tags tag : _children) {
Line 174:             builder.append("," + tag.GetTagIdAndChildrenIds());
builder.append(",").append(tag.GetTagIdAndChildrenIds());
Line 175:         }
Line 176:         return builder;
Line 177:     }
Line 178: 


Line 180:         StringBuilder builder = new StringBuilder();
Line 181:         builder.append(StringFormat.format("'%1$s'", gettag_name()));
Line 182: 
Line 183:         for (tags tag : _children) {
Line 184:             builder.append("," + tag.GetTagNameAndChildrenNames());
builder.append(",").append(tag.GetTagNameAndChildrenNames());
Line 185:         }
Line 186:         return builder;
Line 187:     }
Line 188: 


....................................................
File 
backend/manager/modules/compat/src/main/java/org/ovirt/engine/core/compat/TimeSpan.java
Line 171:         String prefix = "";
Line 172:         if (TotalMilliseconds < 0) {
Line 173:             prefix = "-";
Line 174:         }
Line 175:         return String.format("%s%d.%02d:%02d:%02d.%03d", prefix, 
Days, Hours, Minutes, Seconds, Milliseconds);
Please add comment why this is OK GWT-wise.
Line 176:     }
Line 177: 
Line 178:     public static TimeSpan tryParse(String string) {
Line 179:         try {


....................................................
File 
backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/ADSyntaxChecker.java
Line 291:                          * mark this search as findAll for later use
Line 292:                          */
Line 293:                         findAll = true;
Line 294:                     } else {
Line 295:                         phrase.append(" (" + 
conditionFieldAC.getDbFieldName(so.getBody()));
phrase.append(" (").append(conditionFieldAC.getDbFieldName(so.getBody()));
Line 296:                     }
Line 297:                     break;
Line 298:                 case CONDITION_RELATION:
Line 299:                     /**


Line 313:                          * as it is used in replace.
Line 314:                          */
Line 315:                         phrase.replace("{value}", 
so.getBody().replace("$", "\\$"));
Line 316:                     } else {
Line 317:                         phrase.append(so.getBody() + ")");
phrase.append(so.getBody()).append(")");
Line 318:                     }
Line 319:                     if (nonEqual) {
Line 320:                         retval.append(StringFormat.format("(!%1$s)", 
phrase));
Line 321:                     } else {


....................................................
File 
backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/SyntaxContainer.java
Line 223:         sb.append("Error           = ");
Line 224:         sb.AppendLine(mError.toString());
Line 225:         sb.append("CrossRefObjlist = ");
Line 226:         for (String cro : getCrossRefObjList()) {
Line 227:             sb.append(cro + ", ");
sb.append(cro).append(", ");
Line 228:         }
Line 229:         sb.append("Syntax object list:");
Line 230: 
Line 231:         for (SyntaxObject obj : mObjList) {


Line 245:         sb.append(mError);
Line 246:         sb.append("<BR>Syntax object list:");
Line 247:         sb.append("<BR>CrossRefObjlist = ");
Line 248:         for (String cro : getCrossRefObjList()) {
Line 249:             sb.append(cro + ", ");
sb.append(cro).append(", ");
Line 250:         }
Line 251:         for (SyntaxObject obj : mObjList) {
Line 252:             sb.append("<BR>    ");
Line 253:             sb.append(obj.toString());


....................................................
File 
backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/SyntaxObject.java
Line 41:                 StringFormat.format("body = '%1$s' , startPos = %2$s , 
endPos = %3$s, type = %4$s",
Line 42:                         mBody,
Line 43:                         mPos[0],
Line 44:                         mPos[1],
Line 45:                         mType);
Just formatting, irrelevant to this patch (which is pretty large as is)
Line 46:         return retval;
Line 47:     }


....................................................
Commit Message
Line 6: 
Line 7: core: Removed unnecessary calls to StringFormat & StringHelper
Line 8: 
Line 9: Replaced calls to StringFormat.format to String.format where possible
Line 10: (i.e. not code that compile to GWT) and to simple String concatenation 
where
s/compile/compiles/
Line 11: possible
Line 12: also replaced calls to StringHelper.EqOp calls in
Line 13: String.equals calls in cases that at least one of the args was not null
Line 14: 


Line 7: core: Removed unnecessary calls to StringFormat & StringHelper
Line 8: 
Line 9: Replaced calls to StringFormat.format to String.format where possible
Line 10: (i.e. not code that compile to GWT) and to simple String concatenation 
where
Line 11: possible
missing "." at the end of the sentence.
Line 12: also replaced calls to StringHelper.EqOp calls in
Line 13: String.equals calls in cases that at least one of the args was not null
Line 14: 
Line 15: Added @Override annotation to methods in the edited classes where 
needed


Line 8: 
Line 9: Replaced calls to StringFormat.format to String.format where possible
Line 10: (i.e. not code that compile to GWT) and to simple String concatenation 
where
Line 11: possible
Line 12: also replaced calls to StringHelper.EqOp calls in
s/also/Also/
Line 13: String.equals calls in cases that at least one of the args was not null
Line 14: 
Line 15: Added @Override annotation to methods in the edited classes where 
needed
Line 16: 


Line 9: Replaced calls to StringFormat.format to String.format where possible
Line 10: (i.e. not code that compile to GWT) and to simple String concatenation 
where
Line 11: possible
Line 12: also replaced calls to StringHelper.EqOp calls in
Line 13: String.equals calls in cases that at least one of the args was not null
missing "." at the end of the sentence.
Line 14: 
Line 15: Added @Override annotation to methods in the edited classes where 
needed
Line 16: 
Line 17: Change-Id: I57dc0d9d30f9e5cd4f0beacc8bfd35fe5023afc1


Line 11: possible
Line 12: also replaced calls to StringHelper.EqOp calls in
Line 13: String.equals calls in cases that at least one of the args was not null
Line 14: 
Line 15: Added @Override annotation to methods in the edited classes where 
needed
missing "." at the end of the sentence.
Line 16: 
Line 17: Change-Id: I57dc0d9d30f9e5cd4f0beacc8bfd35fe5023afc1


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I57dc0d9d30f9e5cd4f0beacc8bfd35fe5023afc1
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to