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@xxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@xxxxxxxxxxxxxxxxx |jwilson@xxxxxxxxxx ------- Additional Comments From jwilson@xxxxxxxxxx 2007-07-23 16:44 EST ------- First pass through the spec... 1) Why the use of ExclusiveArch, then list just about all possible arches? I think an ExcludeArch for the arch or arches it doesn't build on might be better. So far as I can see, that list is really just s390/s390x, no? 2) %makeinstall is a big no-no :) http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002 3) the CFLAGS aren't being honored. I've actually got a patch in hand that'll make 'make install DESTDIR=$RPM_BUILD_ROOT' work, as well as the CFLAGS honored: ----8<---- --- wdaemon-0.10/Makefile 2007-07-20 12:07:44.000000000 -0400 +++ wdaemon-0.10.updated/Makefile 2007-07-23 16:30:06.000000000 -0400 @@ -1,4 +1,4 @@ -CFLAGS = -O0 -g -Wall +CFLAGS ?= -O0 -g -Wall OBJS = hotplug.o \ input.o \ monitored.o \ @@ -24,10 +24,10 @@ wdaemon: $(OBJS) gcc $(CFLAGS) -c -o $@ $< install: - mkdir -p $(bindir) - cp wdaemon $(bindir)/ - mkdir -p $(sysconfdir)/rc.d/init.d/ - cp wdaemon.initrd $(sysconfdir)/rc.d/init.d/wdaemon + mkdir -p $(DESTDIR)$(bindir) + cp wdaemon $(DESTDIR)$(bindir)/ + mkdir -p $(DESTDIR)$(sysconfdir)/rc.d/init.d/ + cp wdaemon.initrd $(DESTDIR)$(sysconfdir)/rc.d/init.d/wdaemon clean: rm -f *.o wdaemon core ----8<---- 4) I'd install is_uinput.sh mode 755 instead of 644, which also eliminates the need for the %attr stuff on that file in %files. That's all I've got so far... rpmlint output is fairly clean, just two warnings about the udev rules files not being marked as config files. Not sure yet if they should be, or if we just ignore those. -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review