Spec URL: http://people.redhat.com/jlayton/xwrits/xwrits.spec SRPM URL: http://people.redhat.com/jlayton/xwrits/xwrits-2.22-1.src.rpm Description: Xwrits reminds you to take wrist breaks, which should help you prevent or manage a repetitive stress injury. It pops up an X window when you should rest; you click on that window, then take a break. Xwrits's graphics are brightly colored pictures of a wrist and the attached hand. The wrist clenches and stretches ``as if in pain'' when you should rest, slumps relaxed during the break, and points forward valiantly when the break is over. It is trapped behind bars while the keyboard is locked. Other gestures are included. Extensive command line options let you control how often xwrits appears. It can escalate its behavior over time -- by putting up more flashing windows or actually locking you out of the keyboard, for example -- which makes it harder to cheat. (This is my first package and I'm looking for a sponsor)
Hello Jeffrey, Not yet a proper review, but a couple of points: 1) Use the dist tag: Release: 1%{?dist} 2) Why not use %setup -q, as is standard 3) Use either $RPM_BUILD_ROOT or %{buildroot}, and do it consistently, don't mix and match - choose one and stick to it. 4) There should be a rm -rf $RPM_BUILD_ROOT at the beginning of %install 5) Consider using make %{?_smp_mflags} in build. Fix up these points and I'll run it through a mock build.
Thanks for having a look. I've made the changes you recommended most were pretty straightforward. The only one that wasn't was adding %{?_smp_mflags} to make. I don't have an SMP box handy to test how well that works for this package. I'll plan to test it on one tomorrow though Let me know if you see any other problems.
Tested building the SRPM with %{?_smp_mflags} and it seems to work correctly. I also made a slight change to the .desktop file to make it so that it runs xwrits with some more options.
Greetings. Is the current spec/src.rpm the ones at the beginning of this bug? It's a good idea to increment release numbers and add changelog entries even while the package is under review, so we can check changes against previous revsions... can you post an updated version with the changes you made in comment #2 and #3? I can look at reviewing this in the next few days...
Re-adding comment in that was lost from bugzilla crash: ------- Additional Comments From jlayton 2006-06-11 09:41 EST ------- Sorry for the delay. The current spec/src.rpm were the new ones, but you're correct that I should have bumped the release number. I've done that and uploaded new versions: http://people.redhat.com/jlayton/xwrits/xwrits.spec http://people.redhat.com/jlayton/xwrits/xwrits-2.22-2.src.rpm
I would be happy to review this and potentially sponsor you... you might want to take a look at: http://fedoraproject.org/wiki/Extras/HowToGetSponsored Look for a review coming sometime this weekend...
Sorry I didn't get to this review this weekend... Here's a review. OK - Package name OK - Spec file matches base package name. OK - Meets Packaging Guidelines. OK - License (GPL) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: d49fbcc02dc09d1586281f3b294245da xwrits-2.22.tar.gz d49fbcc02dc09d1586281f3b294245da xwrits-2.22.tar.gz.1 OK - Package compiles and builds on at least one arch. n/a - Package needs ExcludeArch OK - BuildRequires correct n/a - Spec handles locales/find_lang n/a - Spec has needed ldconfig in post and postun n/a - Package is relocatable and has a reason to be. OK - Package owns all the directories it creates. OK - Package has no duplicate files in %files. See below - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Spec has consistant macro usage. OK - Package is code or permissible content. n/a - -doc subpackage needed/used. OK - Packages %doc files don't affect runtime. n/a - Headers/static libs in -devel subpackage. n/a - .pc files in -devel subpackage. n/a - .so files in -devel subpackage. n/a - -devel package Requires: %{name} = %{version}-%{release} n/a - .la files are removed. OK - Package is a GUI app and has a .desktop file OK - Package doesn't own any directories other packages own. See Below - No rpmlint output. Issues: 1. Some rpmlint output: W: xwrits no-version-in-last-changelog Suggest: add "2.22-2" to the end of the * Sun May 28 2006 Jeff Layton <jlayton> line. E: xwrits non-standard-dir-perm /usr/share/doc/xwrits-2.22 0644 Suggest: don't set a default perm in files... %defattr(0644,root,root) should be just %defattr(-,root,root) W: xwrits non-standard-dir-in-usr man Suggest: Don't specify full path for man page: /usr/man/man1/xwrits.1* should be: %{_mandir}/man1/* W: xwrits empty-%post Suggest: remove empty %post section? E: xwrits configure-without-libdir-spec Suggest: add '--libdir=${_libdir}' to the configure step.
Thanks for the review, Kevin. I've made the suggested changes plus a few others to more "macroize" the specfile. The new package and spec file are up on my people page: Spec URL: http://people.redhat.com/jlayton/xwrits/xwrits.spec SRPM URL: http://people.redhat.com/jlayton/xwrits/xwrits-2.22-3.src.rpm Let me know what you think.
All the items in comment #7 are addressed. This package looks good to me now, so it's APPROVED. I am willing to go ahead and sponsor you. You don't have any comments on other review requests, but you have a good history in bugzilla contibuting. ;) You can continue the process by going to: http://fedoraproject.org/wiki/Extras/Contributors on the "Get a Fedora Account" step. Please let me know via email or irc (I'm nirik on #fedora-extras) if you have any questions...
Package is in CVS for devel and FC-5. I've done a build and it was successful. Closing BZ. Thanks for the help, Kevin!