Request for a sponsor and a review of: pam_abl

[Oliver Falk]

Hi Alexander!

On 07/15/2005 02:20 PM, Alexander Dalloz wrote:
> Am Fr, den 15.07.2005 schrieb Oliver Falk um 9:46:
> Starting with a friendly thank you for your review.

My job, isn't it? :-) Just jokein'. You're welcome!

>>Attached is a patch with recommended changes. You don't have to apply 
>>all the whitespace stuff I added, it was just some cosmetic...
>>
>>But:
>>* Macro usage was inconsitant
> 
> Will take care of this in future.

Goooood. :-)

>>* At some points macros weren't used (eg. _sysconfdir}
> 
> Ok, found the note in the packaging guideline, now as you remarks it.
> (Still not all the rules flooding through my veins :)

I guess noone has all rules in his brain. If it would be so, the rpm 
'gurus' wouldn't need a reviewer. :-)

> An additional question regarding macros:
> -should %{name} be used anywhere else than in BuildRoot and if setup
> must use the "-n" switch in %prep? From reading other .spec files I have
> the impression %{name} isn't used wherever possible.

You can use it wherever you need to. I always try to use macros wherever 
possible and it makes sense to do so. For example, it makes no sense to 
use macros in %description or in the Summary tag.

Just have a look at other package and get a 'feeling' for where to use...

> - how about using %{__make}, %{__install} or %{__rm}? Is there a general
> guideline about those macros?

Some people use it, some don't. I heard from some people. That the make 
macro isn't available everywhere and therefor it's better not used. And 
for rm, it usually makes not much sense to use the macro.

But actually, there are many packages that use the above mentioned 
macros (also in FE).

Have a look at /usr/lib/rpm/macros. There are many macros that could be 
used... But maybe it's better to not use ALL of those...

> Noted: both questions not only directed to you Oliver - I appreciate
> advises by anyone willing to teach me.

I hope other people also decide to write you :-)

>>* Added a fine for the reldate (even if it will not live very long)
 >
> Done.
>  
>>* Moved pam_abl from /sbin to /usr/sbin - you don't need this before a 
>>possible external /usr gets mounted.
> 
> Followed your and Tomas' conversation about this. When I originally
> decided for /sbin I did this - like Tomas noted - because pam_tally
> (similar purpose like pam_abl) and others are in /sbin. I see your point
> Oliver and the one is as valid - probably weighting stronger from
> general point of FHS. So: %{_sbindir}.

It's your decision. If you'd prefer /sbin, this would be also OK...

> Renewed src.rpm and spec:
>  http://www.uni-x.org/pam_abl-0.2.2-2.src.rpm
> http://www.uni-x.org/pam_abl.spec

Approved. I'll send out the note in a few minutes.

Best,
  Oliver