Dan Kenigsberg has posted comments on this change.

Change subject: build: Introduce --enable-gluster configuration
......................................................................


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/46708/1/lib/vdsm/constants.py.in
File lib/vdsm/constants.py.in:

Line 22: 
Line 23: from __future__ import absolute_import
Line 24: import os
Line 25: 
Line 26: GLUSTER_ENABLED = False if '@GLUSTER_TRUE@' else True
I find the name a bit misleading, as we always support consumption of gluster 
volumes.

How about GLUSTER_MGMT_ENABLED?
Line 27: 
Line 28: # VDSM management networks
Line 29: LEGACY_MANAGEMENT_NETWORKS = ('ovirtmgmt', 'rhevm')
Line 30: 


https://gerrit.ovirt.org/#/c/46708/1/vdsm.spec.in
File vdsm.spec.in:

Line 13: # Glusterfs package version
Line 14: %global gluster_version 3.7.1
Line 15: 
Line 16: # Glusterfs - overridable using rpmbuild --define "with_gluster 1"
Line 17: %{!?with_gluster: %global with_gluster 0}
why is the default 0? only rhev is expected to want to unset this.

And wouldn't the need to set two flags complicate the life of downstream? I 
like the "automaticness" of the removed code.
Line 18: 
Line 19: # koji build - overridable using rpmbuild --define "fedora_koji_build 
1"
Line 20: %{!?fedora_koji_build: %global fedora_koji_build 0}
Line 21: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I84608625e3b004c64b4928e5325d9375fb952878
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <[email protected]>
Gerrit-Reviewer: Ala Hino <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Bala.FA <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eyal Edri <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Yaniv Dary <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to