https://bugzilla.redhat.com/show_bug.cgi?id=1235305 --- Comment #11 from Ingvar Hagelund <ingvar@xxxxxxxxx> --- (In reply to Jeff Backus from comment #7) > Hi folks, > > I've done a formal review. Here are the highlights, with the formal review > below: Thanks, Jeff New .src.rpm: http://users.linpro.no/ingvar/varnish/hitch/hitch-1.0.0-0.3.3.beta3.fc22.src.rpm New .spec: http://users.linpro.no/ingvar/varnish/hitch/hitch.spec > * BR redhat-rpm-config isn't needed, however, as there is an on-going > discussion about what should go in the minimum buildroot, I won't insist > this is removed. Any reason this was added? It used to be a requirement for hardened build, but that is probably long gone. Fixed. > * Even though BuildRequires allows multiple listings on one line, please > only provide one. Specifically, line 25... Fixed > * Please add comments above %check section explaining why it is disabled by > default and how to use it. Your explanation above is sufficient, just add it > to the .spec file. Fixed > * Missed the -p when installing hitch.conf on line 98. As the file is generated at build time, as was a bit unsure if this was necessary. Fixed > * Please remove the commented %setup macro in %prep. Fixed > * Please remove the commented commands in %build Fixed > * While you're making changes, the description has "It's" but should be > "Its"... :) This was dumped from an upstream description, but still; "It is" -> "Its"??? I'll use "It is", to avoid any confusion. > Overall, looks good. Please address the above and I'll approve it. Great, thanks! Ingvar -- 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