https://bugzilla.redhat.com/show_bug.cgi?id=1209974
--- Comment #5 from Daniel Kopeček dkopecek@redhat.com --- (In reply to Antti Järvinen from comment #2)
Hello Daniel,
I'm not in position to sponsor your package but I made a review about it. Hopefully it will be useful for someone considering sponsorship. Also, I reviewed only this Qt-GUI part, not the devel package from ticket 1209971 as I feel as not being USB-guru but this simple Qt app I do understand.
Package Review
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
Notes:
- While there is LICENSE file in the tarball, there is no indication in source files that LICENSE applies to each of those files. Don't know if this is requirement but it would be nice at least. https://www.gnu.org/licenses/gpl-howto.html has copy-paste examples.
Fixed.
- LICENSE-file mentioned in previous note is not included in resulting binary rpm. Adding a line %license LICENSE
Added.
into section %files should do the trick.
- CXXFLAGS/LDFLAGS may be in conflict, depending on build configuration
Removed the harden build flags. I'll reconsider hardened build enabled using the _hardened_build rpm variable.
- % install section begins with rm -rf $RPM_BUILD_ROOT
- Invalid use of buildroot
Fixed.
- I'd love to see documentation of some kind. While the application itself is very simple, a manpage would not hurt.
Added man page.
- Obviously the icon is also covered by LICENSE?
Changed license in the svg files.
- Wishlist item: in sources usage of tr("..") macro for strings would make it much easier for me to spawn linguist-qt4 and post a finnish language translation to author.
Should be fixed, I've added tr() where appropriate.
Generic: [!]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [!]: Package contains desktop file if it is a GUI application. It has GUI - should count as GUI app? [!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. % install section begins with rm -rf $RPM_BUILD_ROOT ===== SHOULD items =====
Generic: [!]: Buildroot is not present Note: Invalid buildroot found: %{_tmppath}/%{name}%{version}-%{release}-root-%(%{__id_u} -n) See: http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
Fixed.