Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: scanbuttond - Scanner Button tools to SANE https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=209082 ------- Additional Comments From jima@xxxxxxxxxxxxxx 2006-10-04 09:46 EST ------- Yep, that it is. Kind of annoying about executable-marked-as-config-file; thanks for taking that suggestion into consideration (rpmlint showed me, I guess!). Now for the review you earned. :-) Using my own review checklist: http://beer.tclug.org/fedora-extras/review-checklist-1.1.txt 1. rpmlint returns no output for any of the RPMs/SRPM. 2. Package is named as per the Package Naming Guidelines. 3. Spec is scanbuttond.spec 4. Package appears to meet the Packaging Guidelines. 5. Project site listed package as GPL'd. 6. License tag in spec concurs. 7. COPYING included in %doc. 8. Spec file is written in American English. 9. Spec file is legible. 10. Tarball matches upstream, although see below. 11. Package builds fine on FC-5/i386, FC-5/ppc and devel/i386. 12. n/a, unless it doesn't build on x86_64. 13. Package builds in Plague, so I suspect all BuildReqs are fulfilled. 14. Package does not handle locales either properly or improperly. 15. Package runs ldconfig in %post/%postun. However, it doesn't have appropriate Requires(...) lines for /sbin/ldconfig. Please add. 16. Package is not designed to be relocatable. 17. Package does not own /etc/scanbuttond/ -- the change you made in the last iteration removed ownership of this directory. (Sorry, partly my doing.) 18. No duplicate entries in %files. 19. Permissions seem sane; %files has %defattr(...) line. 20. Package has valid %clean section. 21. Macro use appears rather consistent. 22. Package contains code, not content. 23. %doc is small; no -doc subpackage required. 24. %doc does not affect runtime. 25. n/a, no headers or static libraries. 26. n/a, no .pc files. 27. -devel subpackage contains .so files, check. 28. -devel subpackage properly requires the main package. 29. n/a, no .la files. 30. n/a, no GUI app. 31. Package does not own directories/files owned by other packages. 32. Release tag contains %{?dist}. 33. n/a, already includes COPYING. 34. n/a, no translations available, I imagine. 35. Package builds in Plague. 36. I can't verify x86_64, but package builds on i386 and ppc. 37. I'd verify that the package functions as described, but I need a new power supply for my scanner. 38. Unless I'm mistaken, I believe (most) scriptlet commands are supposed to end with "|| :" -- aha: http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-1f750660fb098f31a6a55cb58e752c53dcae39e4 Your 'service scanbuttond condrestart' has it, but 'service scanbuttond stop' is missing the ||; the rest should be fine as-is. (ldconfig and chkconfig seem to be excluded.) 39. n/a, no subpackages besides -devel. As a side note, I'm not sure if it's a blocker, but you only refer to the filename for Source0. I believe it's supposed to be a URL. As it's a Sourceforge-hosted project, it'd likely be one of: http://easynews.dl.sourceforge.net/sourceforge/scanbuttond/scanbuttond-0.2.3.tar.gz (or another mirror) or http://dl.sf.net/scanbuttond/scanbuttond-0.2.3.tar.gz Either of which can be simplified in maintenance with well-placed %{name} & %{version} tags. :-) So, as far as I can see, address #10, #15, #17, and #38, and I think we have a go. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review