Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=922165 --- Comment #7 from Pádraig Brady <pbrady@xxxxxxxxxx> --- (In reply to comment #6) > A few notes and questions : > > - Any reason to not use the systemd macros in %post ? Well I was going to update all the openstack projects together, so was keeping ceilometer consistent with the others for now. Also it introduces compat issues with running this package on Fedora 17. Will consider in future. > > - why does the %post of the api subpackage restart the compute service, and > if that's not a error, why is there no requires from api to compute > subpackages ? typos fixed. > > > - "Patches link to upstream bugs/comments/lists or are otherwise > justified.", there is 2 patch, so the fact they were sent upstream ( or not > ) should be explained. > > especially the 2nd one, called "0002-add-LICENSE.patch", I am not sure that > we can add license as we wish without having legal issues. First is present on all OpenStack packages, and just avoids onlie access when building docs. Upstream is more general and can have online access for builds. The license change is merged upstream, so this patch will drop soon: https://review.openstack.org/#/c/24524/ > > - subpackage requires are wrong, since they need systemdctl and others for > running the command. I've now moved those requires down to common subpackage. > - /var/run/ceilometer must be in tmpfile.d ( > https://fedoraproject.org/wiki/Packaging:Tmpfiles.d ) since /var/run is a > tmpfs We don't really need /var/run/ceilometer actually. Removed. That needs to be done for other Fedora OpenStack services too. > > - why does it requires kombu and qpid ? Isn't it a runtime selection ? Yes, but kombu is a minial dependency and qpid is the platform default. Therefore as per previous discussions and for consistency with other OpenStack packages, I've left this as is. > > - %check is absent, but there is test, any reason to not run them ( or run > only part of them ) ? The OpenStack tests include a lot of dependencies. Again to be looked at enabling at a later stage. > - why is the documentation generation disabled by default ? It's broken. I need to look into it. > - Requires(pre): shadow-utils > this should be on common, since that's where the %post to create the user is > located See above. > - the buildRequires in the doc subpackage are redundant with the main one, > so they could be removed for most of them. BuildRequires left, as they overlap with Requires: not BuildRequires: I think > - the license should be shipped with any combination of subpackages. For > now, there is only a file in the api subpackage. No the main ASL LICENSE is in the common subpackage, while some _separate_ licenses are only applicable to the API logic, and so only shipped in that subpackage. thanks! -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=o3e3onf3dD&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review