Bug 226423 - Merge Review: smartmontools
Summary: Merge Review: smartmontools
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michal Nowak
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 21:00 UTC by Nobody's working on this, feel free to take it
Modified: 2013-03-08 02:03 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-11-26 13:59:17 UTC
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)

Description Nobody's working on this, feel free to take it 2007-01-31 21:00:07 UTC
Fedora Merge Review: smartmontools

http://cvs.fedora.redhat.com/viewcvs/devel/smartmontools/
Initial Owner: tmraz@redhat.com

Comment 1 Jason Tibbitts 2007-06-01 05:34:46 UTC
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 11:13:46 UTC
> 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 11:49:42 UTC
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 08:23:53 UTC
(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 13:15:07 UTC
Any update so far?

Comment 6 Michal Hlavinka 2009-05-25 13:39:20 UTC
(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 15:47:36 UTC
/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 16:12:12 UTC
(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 16:33:22 UTC
OK.

Just verified in CVS. APPROVED.


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