Juan Hernandez has posted comments on this change.

Change subject: restapi: Fix possible version mismatch (#1206068)
......................................................................


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/40085/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetProductVersionQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetProductVersionQuery.java:

Line 16:  */
Line 17: public class GetProductVersionQuery<P extends VdcQueryParametersBase> 
extends QueriesCommandBase<P> {
Line 18: 
Line 19:     public static final String RPM_REG_EX = "^\\d+\\.\\d+\\.\\d+";
Line 20:     public static final String VDC_VERSION_REG_EX = 
"^\\d+\\.\\d+\\.\\d+\\.\\d+";
Just a minor suggestion: since Java 7 you can use named groups in regular 
expressions, so you can do something like this:

  RPM_REG_EX = "^(?<major>\\d+)\\.(?<minor>\\d+)\\.(?<micro>\\d+)";

Later, when you need to access one of the groups you can get it by name:

  Matcher matcher = ...;
  String major = matcher.group("major");
  String minor = matcher.group("minor");
  String micro = matcher.group("micro");

Also the expression can be compiled once, when created, instead of compiling it 
every time it is used:

  public static final Patter RPM_REG_EX = Patter.compile(...);
Line 21: 
Line 22:     public GetProductVersionQuery(P parameters) {
Line 23:         super(parameters);
Line 24:     }


https://gerrit.ovirt.org/#/c/40085/2/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendApiResource.java
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendApiResource.java:

Line 291:     }
Line 292: 
Line 293:     private API addSystemVersion(API api) {
Line 294:         String productVersion = 
getConfigurationValueDefault(String.class,
Line 295:                 ConfigurationValues.ProductRPMVersion);
> Unless there's some additional logic involved, we don't usually use designa
This is easy to solve adding a new "full" or similar property to the backend 
"Version" entity. That way the RESTAPI doesn't need to know anything about RPM 
packaging.

I don't think this is important or urgent, but as we are doing it it is better 
to do it correctly (alson in the capabilities resource).
Line 296:         BrandingManager obrand = BrandingManager.getInstance();
Line 297:         api.setProductInfo(new ProductInfo());
Line 298:         
api.getProductInfo().setName(obrand.getMessage("obrand.backend.product"));
Line 299:         
api.getProductInfo().setVendor(obrand.getMessage("obrand.backend.vendor"));


-- 
To view, visit https://gerrit.ovirt.org/40085
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1bd123c2352db229e52bdbd3f1e2db4ac72018a9
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ori Liel <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Ori Liel <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[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