Bug 677804 - Review Request: drwright - Typing monitor to force typing breaks
Summary: Review Request: drwright - Typing monitor to force typing breaks
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jerry James
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-02-15 21:33 UTC by Christopher Aillon
Modified: 2011-03-17 16:10 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-03-17 16:10:11 UTC
Type: ---
Embargoed:
loganjerry: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Christopher Aillon 2011-02-15 21:33:23 UTC
Spec URL: http://caillon.fedorapeople.org/drwright/drwright.spec
SRPM URL: http://caillon.fedorapeople.org/drwright/drwright-2.91.1-1.fc15.src.rpm
Description: Typing monitor to force typing breaks

Comment 1 Jerry James 2011-03-16 18:44:04 UTC
I'll take this review.

Comment 2 Jerry James 2011-03-16 19:11:50 UTC
MUST items:
- rpmlint output:
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
- package name: OK
- spec file name: OK
- packaging guidelines: OK
- licensing guidelines: OK
- license field: OK
- license file: OK
- American English: OK
- spec file legible: OK
- source file matches upstream: OK, both have md5sum 926e5a7524a218ff662cca2faee9e89f
- builds on at least one primary arch: OK, tried x86_64
- use of ExcludeArch: N/A
- all build requirements in BuildRequires: OK
- use of find_lang: OK
- use of ldconfig: N/A (there are shared libraries, but they're private)
- no copies of system libraries: OK
- relocatable package: N/A
- package owns all directories it creates: OK
- no duplicates in %files: OK
- proper permissions: OK
- consistent use of macros: OK
- code or permissible content: OK
- large documentation in -doc: N/A
- no runtime deps in %doc: OK
- header files in -devel: N/A
- static libraries in -static: N/A
- .so libraries in -devel: N/A (there are .so files, but they're private)
- -devel requires the base package: N/A
- no libtool archives: OK
- GUI application has a .desktop file:

The package review guideline says: "Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section."  This package meets the first part of the requirement, but does not use desktop-file-install.  Is there a reason for that?  If not, please use desktop-file-install.

- do not own files/dirs owned by other packages: OK
- filenames are UTF-8: OK

SHOULD items:
- ask upstream to include license file: N/A
- description and summary include translations: this has not been done.  It is possible to do so by extracting either the "Typing Monitor" or "A computer break reminder." strings from the po files, although I understand that that could be a laborious process.  I won't insist on this.
- package builds in mock: built on Rawhide, but not yet in mock.  Will do that soon and report the results in my next update to this bug.
- package builds on all supported arches: did not test (x86_64 only)
- program functions as described: OK (only did some simple stuff with it)
- sane scriptlets: OK
- subpackages require main package: N/A
- placement of pkgconfig files: N/A
- file dependencies: N/A
- man pages for binaries: N/A (no command-line binaries in this package)

Finally, I think the following are needed:
Requires(post): coreutils glib2
Requires(postun): coreutils glib2 gtk2
Requires(posttrans): gtk2

Comment 3 Jerry James 2011-03-16 19:55:37 UTC
Building in mock failed due to two missing BRs: intltool and libSM-devel.  With those added, the build succeeded.

Comment 4 Jerry James 2011-03-16 20:21:38 UTC
The BR on libtool is unnecessary.  The libtool package is only needed to generate a new libtool script, but this package is using the libtool script distributed with the sources.  The package builds in mock with that BR removed.

Comment 5 Christopher Aillon 2011-03-16 21:40:11 UTC
Original URLs updated.

Added: desktop-file-validate (was an oversight)
Fixed the BRs (they were right initially, I updated to a git snapshot, and then I removed intltool and libSM-devel for some reason and kept libtool instead.  Odd)

I did not add the scriptlet requires per http://fedoraproject.org/wiki/Packaging:ScriptletSnippets

Comment 6 Jerry James 2011-03-16 22:02:12 UTC
(In reply to comment #5)
> Added: desktop-file-validate (was an oversight)
> Fixed the BRs (they were right initially, I updated to a git snapshot, and then
> I removed intltool and libSM-devel for some reason and kept libtool instead. 
> Odd)

That's what package review is for, eh? :-)

> I did not add the scriptlet requires per
> http://fedoraproject.org/wiki/Packaging:ScriptletSnippets

Oh, I see.  Hmmmm, shouldn't the templates on that page check whether the binaries are available instead of just blindly running them, then?  Anyway, that's out of the scope of this review.  All the MUST items are now covered, so I approve this package.

Comment 7 Christopher Aillon 2011-03-16 22:28:08 UTC
New Package SCM Request
=======================
Package Name: drwright
Short Description: Typing monitor to force typing breaks
Owners: caillon
Branches: f15
InitialCC:

Comment 8 Jason Tibbitts 2011-03-17 14:12:30 UTC
Git done (by process-git-requests).

Comment 9 Christopher Aillon 2011-03-17 16:10:11 UTC
Built for f15 and f16.  Closing bug.  Thanks.


Note You need to log in before you can comment on or make changes to this bug.