Bug 772504

I think the package summary does not describe the package very well. The description gave me a completely different view.

Defattr is no longer necessary. If you're not going for EPEL 5, you can drop the clean section, buildroot definition and the rm in the install section.

Please use the name macro, as you did in Source. Looking at your manpage, you forgot a "d" in %{_localstatedir}/lib/btkbd.

There's a macro called sharedstatedir, by the way, that is equivalent to %{_localstatedir}/lib.

The FSF address is outdated.

Why do you want to own %{_sysconfdir}/udev/rules.d?

Why do you require pod2man explicitly?

Thank you for your suggestions. Here are the updated packages:

SPEC: http://v3.sk/~lkundrak/SPECS/btkbdd.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/btkbdd-1.1-2.el6.src.rpm

(In reply to comment #1)
> I think the package summary does not describe the package very well. The
> description gave me a completely different view.

What view did it give you? I can't really think of a better description and would appreciate help here.

> Defattr is no longer necessary. If you're not going for EPEL 5, you can drop
> the clean section, buildroot definition and the rm in the install section.

I choose not to break compatibility with el5 rebuilds for packages that rebuild cleanly. As it gets more obsolete, I'll remove those parts.

> Please use the name macro, as you did in Source. Looking at your manpage, you
> forgot a "d" in %{_localstatedir}/lib/btkbd.

The manpage was actually wrong (the example udev rules wrote to /var/lib/btkbd). However, it makes more sense with two "d"s, so I adjusted that appropriately.

> There's a macro called sharedstatedir, by the way, that is equivalent to
> %{_localstatedir}/lib.

This would break el5 builds:

$ rpm --eval %_sharedstatedir
/usr/com

> The FSF address is outdated.

Changed that to <http://fsf.org>, which seems to be used in GPLv3.

> Why do you want to own %{_sysconfdir}/udev/rules.d?

That was a mistake. Removed it.

> Why do you require pod2man explicitly?

I choose to :)

It is not intuitively obvious (to me) that it is part of perl, nor that it won't change in future or that perl will stay in build group forever. Though all of that is unlikely, I like it to be specified explicitly.

(In reply to comment #2)
> Thank you for your suggestions. Here are the updated packages:
> 
> SPEC: http://v3.sk/~lkundrak/SPECS/btkbdd.spec
> SRPM: http://v3.sk/~lkundrak/SRPMS/btkbdd-1.1-2.el6.src.rpm
> 
> (In reply to comment #1)
> > I think the package summary does not describe the package very well. The
> > description gave me a completely different view.
> 
> What view did it give you? I can't really think of a better description and
> would appreciate help here.
> 

The way you put it now sounds a lot better to me! :)

> > Defattr is no longer necessary. If you're not going for EPEL 5, you can drop
> > the clean section, buildroot definition and the rm in the install section.
> 

(Defattr is not necessary for EPEL 5 either.)

> I choose not to break compatibility with el5 rebuilds for packages that rebuild
> cleanly. As it gets more obsolete, I'll remove those parts.
> 
> > Please use the name macro, as you did in Source. Looking at your manpage, you
> > forgot a "d" in %{_localstatedir}/lib/btkbd.
> 
> The manpage was actually wrong (the example udev rules wrote to
> /var/lib/btkbd). However, it makes more sense with two "d"s, so I adjusted that
> appropriately.
> 
> > There's a macro called sharedstatedir, by the way, that is equivalent to
> > %{_localstatedir}/lib.
> 
> This would break el5 builds:
> 
> $ rpm --eval %_sharedstatedir
> /usr/com
> 
Ah, I wasn't aware of that.

> > The FSF address is outdated.
> 
> Changed that to <http://fsf.org>, which seems to be used in GPLv3.

No, I meant the postal address. They moved from Temple Place to Franklin Street in 2005.

> > Why do you want to own %{_sysconfdir}/udev/rules.d?
> 
> That was a mistake. Removed it.
> 
> > Why do you require pod2man explicitly?
> 
> I choose to :)
> 
> It is not intuitively obvious (to me) that it is part of perl, nor that it
> won't change in future or that perl will stay in build group forever. Though
> all of that is unlikely, I like it to be specified explicitly.

I tried your software but messed up somehow. Let's see if I can get it working for me! :)

(In reply to comment #3)
> (In reply to comment #2)
> > Thank you for your suggestions. Here are the updated packages:
> > 
> > SPEC: http://v3.sk/~lkundrak/SPECS/btkbdd.spec
> > SRPM: http://v3.sk/~lkundrak/SRPMS/btkbdd-1.1-2.el6.src.rpm
> > 
> > (In reply to comment #1)
> > > I think the package summary does not describe the package very well. The
> > > description gave me a completely different view.
> > 
> > What view did it give you? I can't really think of a better description and
> > would appreciate help here.
> > 
> 
> The way you put it now sounds a lot better to me! :)

Oh, I reworded the summary. I thought you were referring to the description.

> > > Defattr is no longer necessary. If you're not going for EPEL 5, you can drop
> > > the clean section, buildroot definition and the rm in the install section.
> > 
> 
> (Defattr is not necessary for EPEL 5 either.)

rpmlint whines then :(
Would you mind if I leave it there just to make el6 rpmlint (that I use) happy?
 
> > > The FSF address is outdated.
> > 
> > Changed that to <http://fsf.org>, which seems to be used in GPLv3.
> 
> No, I meant the postal address. They moved from Temple Place to Franklin Street
> in 2005.

Yes. The GPLv3 text does not include the postal address, just an URL, thus I just leave just an URL here as well.

> I tried your software but messed up somehow. Let's see if I can get it working
> for me! :)

Let me know then please. Be sure not to skip manual's BUGS section.

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Thank you for your suggestions. Here are the updated packages:
> > > 
> > > SPEC: http://v3.sk/~lkundrak/SPECS/btkbdd.spec
> > > SRPM: http://v3.sk/~lkundrak/SRPMS/btkbdd-1.1-2.el6.src.rpm
> > > 
> > > (In reply to comment #1)
> > > > I think the package summary does not describe the package very well. The
> > > > description gave me a completely different view.
> > > 
> > > What view did it give you? I can't really think of a better description and
> > > would appreciate help here.
> > > 
> > 
> > The way you put it now sounds a lot better to me! :)
> 
> Oh, I reworded the summary. I thought you were referring to the description.
> 
> > > > Defattr is no longer necessary. If you're not going for EPEL 5, you can drop
> > > > the clean section, buildroot definition and the rm in the install section.
> > > 
> > 
> > (Defattr is not necessary for EPEL 5 either.)
> 
> rpmlint whines then :(
> Would you mind if I leave it there just to make el6 rpmlint (that I use) happy?

No, just keep it, it doesn't hurt!
> 
> > > > The FSF address is outdated.
> > > 
> > > Changed that to <http://fsf.org>, which seems to be used in GPLv3.
> > 
> > No, I meant the postal address. They moved from Temple Place to Franklin Street
> > in 2005.
> 
> Yes. The GPLv3 text does not include the postal address, just an URL, thus I
> just leave just an URL here as well.

I see!
> 
> > I tried your software but messed up somehow. Let's see if I can get it working
> > for me! :)
> 
> Let me know then please. Be sure not to skip manual's BUGS section.

Hm, I still don't manage, nevermind! :)

> > > I tried your software but messed up somehow. Let's see if I can get it working
> > > for me! :)
> > 
> > Let me know then please. Be sure not to skip manual's BUGS section.
> 
> Hm, I still don't manage, nevermind! :)

I am quite interested, since you probably are second person (apart from me) that uses the thing and I really intended to make it 'just work' :)

Feel free to reach me at IRC (lkundrak at freenode) or drop a mail at lubo.rintel if you don't feel like polluting this review request.

This package looks safe and match with latest guidelines. It has proper licence, spec file looks safe enough. Also please don't forget to remove BuildRoot if you are willing to build this package for Fedora.

Also works nicely with my iPad (/me hides). Good job Lubomir!

review+

New Package SCM Request
=======================
Package Name: btkbdd
Short Description: Software bluetooth keyboard
Owners: lkundrak
Branches: f15 f16 el6 f17

Git done (by process-git-requests).

Michal, please take ownership of review BZs, thanks!

Imported and build.
Thank you!