Bug 226423

Why is the "fedora-review" flag set on this?  If someone's reviewing this
package, they should assign it to themselves and change the status to ASSIGNED.

> License:        GPLv2+

Missing license in: cciss.cpp cciss.h

> BuildRoot:      %{_tmppath}/%{name}-%{version}-root

https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag

> PreReq:         /sbin/chkconfig /sbin/service

1) https://fedoraproject.org/wiki/Packaging:Guidelines#PreReq
2) Better use the ``pkgname'' instead of ``/some/path/app''

> BuildRequires: readline-devel ncurses-devel /usr/bin/aclocal /usr/bin/automake /usr/bin/autoconf util-linux groff gettext

Better use the ``pkgname'' instead of ``/some/path/app''

> Obsoletes:      kernel-utils smartmontools-config

Still necessary?

> ExclusiveArch:  i386 x86_64 %{arm} ia64 ppc ppc64

F-11 is going to be i586, %{ix86} is your friend.

> %preun
> if [ "$1" = "0" ] ; then
>  /sbin/service smartd stop >/dev/null 2>&1
>  /sbin/chkconfig --del smartd
> fi
> exit 0
`-^^^^^^

I guess it's not necessary in recent Fedora.

https://fedoraproject.org/wiki/Packaging:Guidelines#Scriptlets

> %triggerpostun -- kernel-utils
> /sbin/chkconfig --add smartd
> exit 0

``kernel-utils'' does not seeems to be present in Fedora anymore. Dump it?

> %defattr(-,root,root)

Use %defattr(-,root,root,-)

> %{_sysconfdir}/rc.d/init.d/smartd

Use:   %{_initrddir}         

# %ghost %verify(not md5 size mtime) %config(noreplace) %{_sysconfdir}/smartd.conf

> Dump it.

almost done, remains only

> > License:        GPLv2+
>
> Missing license in: cciss.cpp cciss.h

emailed upstream, waiting for response

(In reply to comment #3)
> almost done, remains only
> 
> > > License:        GPLv2+
> >
> > Missing license in: cciss.cpp cciss.h
> 
> emailed upstream, waiting for response  

no response and no change in their cvs, I'll ping them again...

Any update so far?

(In reply to comment #5)
> Any update so far?  

no, still no response and no change in the cvs

/smartmontools-5.38-24.20091119svn.fc12.i686.rpm /home/newman/rpmbuild/RPMS/i686/smartmontools-debuginfo-5.38-24.20091119svn.fc12.i686.rpm 
smartmontools.i686: W: incoherent-init-script-name smartd ('smartmontools', 'smartmontoolsd')
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

(Now installed the new smartmontools.)

newman ~ $ rpmlint smartmontools
smartmontools.i686: W: incoherent-init-script-name smartd ('smartmontools', 'smartmontoolsd')
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

I guess this is OK.

> BuildRequires:  /usr/bin/aclocal

Should be better automake.

> Source0:        http://prdownloads.sourceforge.net/%{name}/%{name}-%{version}.snap20091119.tar.gz

I don't see such file there. Just keep the filename part. https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control

> Version:        5.38
> Release:        24.20091119svn%{?dist}
> [...]
> %setup -q -n %{name}-5.39

Shouldn't we have "Version: 5.39" + "Release: 0.20091119svn%{?dist}", since the tarball's version is "5.39"? I'd rather call it 5.39 pre-release than 5.38 release bump.

(In reply to comment #7)
> > BuildRequires:  /usr/bin/aclocal
> 
> Should be better automake.
> 

fixed

> > Source0:        http://prdownloads.sourceforge.net/%{name}/%{name}-%{version}.snap20091119.tar.gz
> 
> I don't see such file there. Just keep the filename part.
> https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control

fixed

> > Version:        5.38
> > Release:        24.20091119svn%{?dist}
> > [...]
> > %setup -q -n %{name}-5.39
> 
> Shouldn't we have "Version: 5.39" + "Release: 0.20091119svn%{?dist}", since the
> tarball's version is "5.39"? I'd rather call it 5.39 pre-release than 5.38
> release bump.  

5.39 in setup section is there because smartmontools package is created by "make dist" and (as usual) upstream bumbs version in autotools config files just after releasing any new version. I think it's better to use snapshot versioning as specified in:

https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages

OK.

Just verified in CVS. APPROVED.