https://bugzilla.redhat.com/show_bug.cgi?id=1205489 Michael Scherer <misc@xxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |misc@xxxxxxxx --- Comment #6 from Michael Scherer <misc@xxxxxxxx> --- So, a few notes: 1) %{_sharedstatedir}/edeploy/ seems to be unowned 2) I prefer to have 1 requires on each line ( easier to see with diff, but just a personal preference, given for the pleasure of being pedantic as I promised to be ) 3) no need for BuildArch: noarch on the -devel package, that's redundant with the main rpm. 4) there is no file who details the license in a %license, please add it. 5) %{_datadir}/%{name} is unowned 6) %attr(0644, apache, apache) %{_sharedstatedir}/edeploy/*.cmdb this would requires a user apache on the system, and it is created by another rpm, which in turn should be required, if I am not wrong. I also suspect this might requires some selinux policy change, but that's outside of this review and package. In fact, if the files can be modified by apache, they should be marked as %ghost or something, and if they can't, then they shouldn't belong to apache, no ? Could you clarify that part ? 7) any reason to not run the tests in %check ( since there is tox.ini ) ? if so, I think we should have a comment in the rpm saying why they are not enabled, and if we can run, then run them. I might have others stuff to see once I look more on the policy for web application, and dig a bit more in the source code. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review