https://bugzilla.redhat.com/show_bug.cgi?id=844011 Michael Scherer <misc@xxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |misc@xxxxxxxx --- Comment #2 from Michael Scherer <misc@xxxxxxxx> --- I was redacting the comment for the previous bug when you changed the name, but since the file is the same : Hi, So a few issues : /usr/libexec/stickshift/cartridges/abstract is the only directory, but /usr/libexec/stickshift/cartridges and /usr/libexec/stickshift/ would end unowned So either a dependency is missing, or the file should be owned by the rpm, cf https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership [!]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. Note: rm -rf is only needed if supporting EPEL5 ( as i guess openshift would not go to EPEL , this should be cleaned ) I think the license should also be placed in /usr/share/doc, as this is where people would seek it ( but not blocking, afaik ). I also think it would be better to have the license in each file, but that's nitpicking I am also wondering if the requires are correct. Git is likely used by stickshift_abstract script, but why mod_ssl ? Looking at a few file, there is also missing requires : abstract/info/bin/jenkins_build use ruby, rubygems, json abstract/info/connection-hooks/publish-http-url use facter, python abstract/info/connection-hooks/publish-gear-endpoint too ( and in fact, there is code duplication ) abstract/info/connection-hooks/nurture_app_push.sh use curl abstract/info/connection-hooks/open_ports.sh use socat and facter abstract/info/connection-hooks/sync_gears.sh use rsync abstract/info/hooks/deploy-httpd-proxy use rhc-idler, not sure where it come from abstract/info/lib/apache run various apache stuff with service, abstract/info/lib/network run lsof abstract-httpd/info/bin/app_ctl.sh run httpd directly abstract-jboss/info/bin/build.sh requires mvn, from maven I looked at all files, but I may have look one so I guess a double check would not be bad. I am also not [!]: SHOULD Packages should try to preserve timestamps of original installed files. I think you need to use a specific option of cp ( -p or -a ) [!]: SHOULD Spec use %global instead of %define. Note: %define cartdir %{_libexecdir}/stickshift/cartridges again, not blocking but there is subtle effect https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define And there is some compiled code in abstract-jboss/info/data/mysql.tar, and I am not sure if this would be a exception ( https://fedoraproject.org/wiki/Packaging:Guidelines#No_inclusion_of_pre-built_binaries_or_libraries ). -- You are receiving this mail because: You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review