[Bug 249059] Review Request: wdaemon - hotplug helper for wacom x.org driver

[bugzilla at redhat.com]

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: wdaemon - hotplug helper for wacom x.org driver


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=249059


jwilson at redhat.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+




------- Additional Comments From jwilson at redhat.com  2007-07-24 11:16 EST -------
Fedora Package Review: wdaemon
------------------------------

MUST Items:

* rpmlint output acceptable (post full output w/waiver notes where needed): 
  $ rpmlint /build/RPMS/x86_64/wdaemon-*0.10-2*
  W: wdaemon non-conffile-in-etc /etc/udev/rules.d/61-uinput-wacom.rules
  W: wdaemon non-conffile-in-etc /etc/udev/rules.d/61-uinput-stddev.rules

  I'm thinking it wouldn't hurt to just mark these %config(noreplace), in the
event a user does go and edit them/append to them. It completely silences
rpmlint if we go that route, and I don't see any real reason not to just do it.

* Meets Package Naming Guidelines: PASS

* spec file name matches %{name}, in the format %{name}.spec (nb: there are a
few exceptions): PASS

* The package must meet the Packaging Guidelines: PASS

* open-source compatible license and meets fedora legal reqs: PASS

* License field in spec matches actual license: PASS 

* If source includes text of license(s) in its own file, that file must be in
%doc: PASS

* spec file legible and in American English: PASS

* sources used match the upstream source, as provided in spec URL. Verify with
md5sum (if no upstream URL, source creation method must be documented and can be
verified using diff): PASS

  $ md5sum wdaemon-0.10.tar.bz2*
  9c90cefbe4ae7d6c79ded408f9a435a7  wdaemon-0.10.tar.bz2
  9c90cefbe4ae7d6c79ded408f9a435a7  wdaemon-0.10.tar.bz2.1

* produces binary rpms on at least one arch: PASS (f7/x86_64)

* If ExcludeArch used, must be documented why (and a bug filed against ExArch
tracker once approved): NEEDS WORK

  Generally, we're to assume the package will build on any architecture, and
only exclude it from building on a certain arch if there's a good reason to do
so. ExclusiveArch is really frowned upon, unless its a package that really only
makes sense on a very limited set or arches.

* BuildRequires are sane: PASS

* locales, if necessary, handled properly with %find_lang: N/A

* if package contains shared libs, calls ldconfig in %post/postun: N/A

* if package is relocatable, must justify: N/A

* package owns all directories it creates: PASS

* no duplicates in %files: PASS

* Permissions on %files sane: PASS -- though I might suggest some further
updates to the Makefile to use 'install -mXXXX' to install the binaries and init
script, rather than having to use %attr in the %files section.

* %clean includes rm -rf %{buildroot}/$RPM_BUILD_ROOT: PASS

* macros used consistently: PASS

* package contains code, or permissable content: PASS

* Large/lots of docs, if present, should go in a -doc subpackage: N/A

* files in %doc aren't required for package to work: PASS

* Header files in -devel package: N/A

* Static libs in -static package: N/A

* package Reqs: pkgconfig if pkgconfig(.pc) files present: N/A

* if package has versioned libs, unversioned ones go in -devel package: N/A

* if present, -devel packages must require the base package NVR (w/some rare
exceptions):  N/A

* no libtool archives (w/some rare exceptions): PASS

* if GUI app, include a %{name}.desktop file, installed with
desktop-file-install in the %install section (or justify why not): N/A

* don't own files or folders other package own (or justify why you must): PASS

* %install starts with rm -rf %{buildroot}/$RPM_BUILD_ROOT: PASS

* filenames in packages must be valid UTF-8: PASS


SHOULD Items (not absolutely mandatory, but highly encouraged)

* If source does not include license text(s), ask upstream to include it: N/A
(already included)

* description and summary sections in spec should contain translations for
supported Non-English languages, if available: N/A

* package should build in mock: PASS (f7/x86_64)

* package should build on all supported architectures: not tested

* package should function as expected: don't have hardware to test myself

* any scriptlets must be sane: N/A

* subpackages other than -devel require the base package using a fully versioned
dependency: N/A

* pkgconfig files go in -devel pkg, unless package is a devel tool itself: N/A

* If package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or
/usr/sbin consider requiring the package which provides the file instead of the
file itself: N/A

------------------------------

So for the short version, I'd add %config(noreplace) for the two udev rules
files, since it looks like it only helps and use ExcludeArch: if you really need
to have this not build on a specific arch. Although I suppose that could start
to get equally messy (mips, arm, s390, s390x...). Okay, if you do want to stick
with ExcludeArch, I suppose that's fine. One note though: do you really want to
exclude ppc64? Only other minor spec cleanup I'd suggest is to remove the
extraneous slashes between $RPM_BUILD_ROOT and %{_sysconfdir} on lines 36 and 37.

Package APPROVED, none of the above are blockers to me.

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.