Juan Hernandez has posted comments on this change.
Change subject: packaging: use yum API
......................................................................
Patch Set 3: (5 inline comments)
....................................................
File packaging/fedora/setup/common_utils.py
Line 224: # Call xpathEval to strip the metadata string from the top of
the new xml node
Line 225: parentNode.addChild(newNode.xpathEval('/*')[0])
Line 226:
Line 227:
Line 228: class MiniYumSink(object):
A comment describing why this is needed and what it does would help.
Line 229:
Line 230: def _currentfds(self):
Line 231: fds = []
Line 232: try:
Line 269: save = self._currentfds()
Line 270: for i in range(3):
Line 271: os.dup2(self._fds[i], i)
Line 272: print output_messages.WARN_INSTALL_GPG_KEY % (userid,
hexkeyid)
Line 273: ret = askYesNo(INFO_Q_PROCEED)
Does this mean that the dialog with the user will change somewhere? We will be
asking new questions? If that is the case please remember that we may have to
update the documentation as well.
Line 274: for i in range(3):
Line 275: os.dup2(save[i], i)
Line 276: return ret
Line 277:
....................................................
File packaging/fedora/setup/engine-upgrade.py
Line 545:
Line 546: return False
Line 547:
Line 548: def main(options):
Line 549: # BEGIN: PROCESS-INITIALIZATION
These comments seem to be intended for a machine. What do they mean? Just
curious.
Line 550: miniyumsink = utils.MiniYumSink()
Line 551: MiniYum.setup_log_hook(sink=miniyumsink)
Line 552: extraLog = open(LOG_FILE, "a")
Line 553: miniyum = MiniYum(sink=miniyumsink, extraLog=extraLog)
....................................................
File packaging/fedora/setup/output_messages.py
Line 186: INFO_STRING_EXCEEDS_MAX_LENGTH="String length exceeds the maximum
length allowed: %s"
Line 187: INFO_STRING_CONTAINS_ILLEGAL_CHARS="String contains illegal
characters"
Line 188:
Line 189: # Yum
Line 190: WARN_INSTALL_GPG_KEY = "\nApprove installing GPG key userid=%s
hexkeyid=%s"
This message looks a bit hard for an average user? Do you think that people
will understand why do they need to approve that? This will need to be
documented.
Line 191:
Line 192: #####################
Line 193: #####ERR MESSAGES####
Line 194: #####################
....................................................
File packaging/fedora/spec/ovirt-engine.spec.in
Line 745: %{engine_data}/scripts/setup_params.py*
Line 746: %{engine_data}/scripts/setup_sequences.py*
Line 747: %{engine_data}/scripts/setup_controller.py*
Line 748: %{engine_data}/scripts/common_utils.py*
Line 749: %{engine_data}/scripts/miniyum.py*
This reminds me that we should probably add "Requires: yum" to the "setup"
package. Not that it will fix any problem, but it is good practice to have the
requirement explicitly.
Line 750: %{engine_data}/scripts/output_messages.py*
Line 751: %{engine_data}/scripts/nfsutils.py*
Line 752: %{engine_data}/scripts/engine-setup.py*
Line 753: %{engine_data}/scripts/engine-cleanup.py*
--
To view, visit http://gerrit.ovirt.org/8221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I01c0c10c3bca42770bf05222c8b2b82b01fee0a6
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Kiril Nesenko <[email protected]>
Gerrit-Reviewer: Moran Goldboim <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
