Matthias Heinz has uploaded a new change for review.
Change subject: This patch adds some basic debian support ......................................................................
This patch adds some basic debian support
Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff --- M vdsm/caps.py M vdsm/clientIF.py 2 files changed, 46 insertions(+), 14 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/10/1110/1 -- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com
Dan Kenigsberg has posted comments on this change.
Change subject: This patch adds some basic debian support ......................................................................
Patch Set 1: (2 inline comments)
.................................................... File vdsm/caps.py Line 58: DEB = 'Debian' Would Ian be offended?
.................................................... File vdsm/clientIF.py Line 1029: caps.OSName.RHEVH, caps.OSName.DEB): gah, FEDORA should be here, too!
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com
Itamar Heim has posted comments on this change.
Change subject: This patch adds some basic debian support ......................................................................
Patch Set 1: (1 inline comment)
i don't feel strongly about the 2nd part of the comment, especially if we don't expect too many more implementations. but moving the key packages to a config per distro seems more robust to future changes
.................................................... File vdsm/caps.py Line 260: if getos() in (OSName.RHEVH, OSName.OVIRT, OSName.FEDORA, OSName.RHEL): doesn't this if statment smells of a config item of: 1. list of key packages per distribution. 2. name of pakcage implementation to use to query?
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: This patch adds some basic debian support ......................................................................
Patch Set 1: Looks good to me, but someone else must approve
Itamar, I do not see why a config is any better than in-code python definition. I don't expect end users to play with the list of Key Packages
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com
Itamar Heim has posted comments on this change.
Change subject: This patch adds some basic debian support ......................................................................
Patch Set 1:
i guess a matter of taste. my view it is kind of a magic number. you want to be able to change it via config rather than via code. could call it a property file i guess i agree it is not something user should change (in engine we use config for anything which change should not require a code change, but not all are user changeable). my point is tomorrow you will want to add another list of packages per another distro - you shouldn't need to change your code for that.
but i don't feel strongly about it
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: This patch adds some basic debian support ......................................................................
Patch Set 1:
Itamar, I don't quite like premature generalizations out of 2 cases. In my opinion, it can safely wait for the Gentoo guy who'd like to run vdsm.
Matthias, since I do not yet know you or your affiliation, I would like to confirm that all Vdsm code, including yours, is under GPLv2+ license. Also, I'd be happy if your submission includes an update to the AUTHORS files.
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com
Matthias Heinz has posted comments on this change.
Change subject: This patch adds some basic debian support ......................................................................
Patch Set 1: (1 inline comment)
Yes, I've got the permission from my boss to contribute this patch.
.................................................... File vdsm/caps.py Line 58: DEB = 'Debian' Who's Ian and why would he be offended?
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Matthias Heinz matthias.heinz@goobernetworks.com
Dan Kenigsberg has posted comments on this change.
Change subject: This patch adds some basic debian support ......................................................................
Patch Set 1: (1 inline comment)
.................................................... File vdsm/caps.py Line 58: DEB = 'Debian' http://en.wikipedia.org/wiki/Ian_Murdock#cite_note-0
you've left him out of DEBIAN
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Itamar Heim iheim@redhat.com Gerrit-Reviewer: Matthias Heinz matthias.heinz@goobernetworks.com
Dan Kenigsberg has posted comments on this change.
Change subject: This patch adds some basic debian support ......................................................................
Patch Set 2: Verified; Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Matthias Heinz matthias.heinz@goobernetworks.com
Douglas Schilling Landgraf has posted comments on this change.
Change subject: This patch adds some basic debian support ......................................................................
Patch Set 2: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Matthias Heinz matthias.heinz@goobernetworks.com
Dan Kenigsberg has posted comments on this change.
Change subject: This patch adds some basic debian support ......................................................................
Patch Set 2:
Matthias, since I've touched your patch, I'd be happy if you come over and ACK it.
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Matthias Heinz matthias.heinz@goobernetworks.com
Matthias Heinz has posted comments on this change.
Change subject: This patch adds some basic debian support ......................................................................
Patch Set 2: Looks good to me, but someone else must approve
Your changes look good to me
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Matthias Heinz matthias.heinz@goobernetworks.com
Dan Kenigsberg has posted comments on this change.
Change subject: This patch adds some basic debian support ......................................................................
Patch Set 2: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Matthias Heinz matthias.heinz@goobernetworks.com
Dan Kenigsberg has submitted this change and it was merged.
Change subject: This patch adds some basic debian support ......................................................................
This patch adds some basic debian support
Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff --- M vdsm/caps.py M vdsm/clientIF.py 2 files changed, 48 insertions(+), 15 deletions(-)
Approvals: Douglas Schilling Landgraf: Looks good to me, but someone else must approve Matthias Heinz: Looks good to me, but someone else must approve Dan Kenigsberg: Verified; Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/1110 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: Ie021cd9f64f80fde2442b6ad7c75c0b5b1c746ff Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Matthias Heinz matthias.heinz@goobernetworks.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Matthias Heinz matthias.heinz@goobernetworks.com
vdsm-patches@lists.fedorahosted.org