Bug 226423 - Merge Review: smartmontools
Merge Review: smartmontools
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michal Nowak
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 16:00 EST by Nobody's working on this, feel free to take it
Modified: 2013-03-07 21:03 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-11-26 08:59:17 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mnowak: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 16:00:07 EST
Fedora Merge Review: smartmontools

http://cvs.fedora.redhat.com/viewcvs/devel/smartmontools/
Initial Owner: tmraz@redhat.com
Comment 1 Jason Tibbitts 2007-06-01 01:34:46 EDT
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.
Comment 2 Michal Nowak 2009-03-02 06:13:46 EST
> 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.
Comment 3 Michal Hlavinka 2009-03-10 07:49:42 EDT
almost done, remains only

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

emailed upstream, waiting for response
Comment 4 Michal Hlavinka 2009-04-20 04:23:53 EDT
(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...
Comment 5 Michal Nowak 2009-05-25 09:15:07 EDT
Any update so far?
Comment 6 Michal Hlavinka 2009-05-25 09:39:20 EDT
(In reply to comment #5)
> Any update so far?  

no, still no response and no change in the cvs
Comment 7 Michal Nowak 2009-11-25 10:47:36 EST
/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.
Comment 8 Michal Hlavinka 2009-11-25 11:12:12 EST
(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
Comment 9 Michal Nowak 2009-11-25 11:33:22 EST
OK.

Just verified in CVS. APPROVED.

Note You need to log in before you can comment on or make changes to this bug.