https://bugzilla.redhat.com/show_bug.cgi?id=1235305
--- Comment #11 from Ingvar Hagelund ingvar@linpro.no --- (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...
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