Alon Bar-Lev has posted comments on this change.

Change subject: packaging: setup: Force a minimal ETL version
......................................................................


Patch Set 5: Code-Review+1

(1 comment)

minor note regarding the boolean expression.

thanks!

http://gerrit.ovirt.org/#/c/27524/5/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine-dwh/core/check_etl.py
File 
packaging/setup/plugins/ovirt-engine-setup/ovirt-engine-dwh/core/check_etl.py:

Line 80:         )
Line 81:         if (
Line 82:             (int(odwhcons.Const.VERSION_MAJOR) != int(minMajor)) or
Line 83:             (int(odwhcons.Const.VERSION_MINOR) != int(minMinor)) or
Line 84:             (int(odwhcons.Const.VERSION_PATCH_LEVEL) < 
int(minPatchLevel))
aha! this is confusing... again because it is not minimal... :)

proper sequence should be, but it is not proper minimal check.

 major != major or
 (major == major and minor != minor) or
 (major == major and minor == minor and level < levl)

I suggest using the positive way...

 not (
     major == major and
     minor == minor
     fix >= fix
 )
Line 85:         ):
Line 86:             raise RuntimeError(
Line 87:                 _(
Line 88:                     'Minimal supported DWH version ({minimal}) is '


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iccef80d1397c6b66ad5a8440e59af238b42416a7
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-dwh
Gerrit-Branch: master
Gerrit-Owner: Yedidyah Bar David <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Simone Tiraboschi <[email protected]>
Gerrit-Reviewer: Yaniv Dary <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[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