Simone Tiraboschi has posted comments on this change.

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


Patch Set 6:

(2 comments)

http://gerrit.ovirt.org/#/c/27524/6/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 not (
Line 82:             (int(odwhcons.Const.VERSION_MAJOR) == int(minMajor)) and
Line 83:             (int(odwhcons.Const.VERSION_MINOR) == int(minMinor)) and
Line 84:             (int(odwhcons.Const.VERSION_PATCH_LEVEL) >= 
int(minPatchLevel))
> These versions are the setup version or the dwh version? 
On my opinion minMajor and minMinor names are a bit confusing: if we are 
exacting requiring version minMajor.minMinor release and no more, it's not the 
minor: it's just the required or the supported release
Line 85:         ):
Line 86:             raise RuntimeError(
Line 87:                 _(
Line 88:                     'Minimal supported DWH version ({minimal}) is '


Line 86:             raise RuntimeError(
Line 87:                 _(
Line 88:                     'Minimal supported DWH version ({minimal}) is '
Line 89:                     'incompatible with installed package version '
Line 90:                     '({major}.{minor}.{patch_level})'
> This message is not clear enough. You should at least write that it's the m
Also here: if we don't accept anything different also if major it seams a bit 
confusing calling it 'Minimal supported DWH version'
Line 91:                 ).format(
Line 92:                     minimal=minimalVersion,
Line 93:                     major=odwhcons.Const.VERSION_MAJOR,
Line 94:                     minor=odwhcons.Const.VERSION_MINOR,


-- 
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: 6
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