https://bugzilla.redhat.com/show_bug.cgi?id=1288731 --- Comment #7 from awilliam@xxxxxxxxxx <awilliam@xxxxxxxxxx> --- Thanks for the review! As a general note: the spec is based on the openSUSE spec, so most of the stuff you mention comes from that (e.g. the find commands). "Fedora <= 21 is no more, so some conditions in the spec file can be simplified." - well...you can't just drop the Fedora bit because then the conditional would always fail for Fedora builds. We could just make it '%if 0%{?fedora} || 0%{?rhel} > 7', I guess, but that doesn't seem like a huge improvement. I think I'm just gonna leave it for now. openSUSE still has Requires: git-core , but looking through the code I actually don't think there is any use of git at runtime any more, so I'm just going to drop that dep entirely. The find stuff can be improved indeed (these again come direct from the openSUSE spec), I don't think I want to use NO_PACKLIST in case I ever manage to get all the bits built for EPEL 7. I have no idea whether the second 'find and destroy' command is even needed any more, I guess I should look and see what it actually does. There are different philosophies on the file list - just including the entire directory is simpler but means you don't necessarily notice big changes to the package loadout. I have my own preferences, but at this point I'd actually like to simply follow what the openSUSE folks do, because it makes it easier to track their changes in our package. I assume upstream has a reason to not mark the dbus config file as (noreplace), but OTOH we don't use any of that stuff in our openQA and I don't really care about it, so I'm fine with changing it :P "- Package contains BR: python2-devel or python3-devel" - er, what? 'python' does not appear anywhere in the spec, nor is there any python in the package, so I don't know what this is about. Revised spec and SRPM: https://www.happyassassin.net/reviews/os-autoinst/os-autoinst.spec https://www.happyassassin.net/reviews/os-autoinst/os-autoinst-4.2.1-6.fc24.src.rpm -- 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