https://bugzilla.redhat.com/show_bug.cgi?id=1209974 --- Comment #5 from Daniel Kopeček <dkopecek@xxxxxxxxxx> --- (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. -- 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