Bug 505928 - Review Request: hdapsd - Protects hard drives by parking head when fall is detected
Review Request: hdapsd - Protects hard drives by parking head when fall is de...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-14 14:55 EDT by Tomasz Torcz
Modified: 2009-11-04 04:35 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-09-04 13:00:12 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Tomasz Torcz 2009-06-14 14:55:09 EDT
Spec URL: https://pipebreaker.pl/~zdzichu/pkgs/hdapsd.spec
SRPM URL: https://pipebreaker.pl/~zdzichu/pkgs/hdapsd-20090401-1.fc11.src.rpm
Description: 
This is a disk protection user-space daemon. It monitors the acceleration
values through the HDAPS/AMS interfaces and automatically initiates disk head
parking if a fall or sliding of the laptop is detected.

HDAPS is typically found in ThinkPad laptops and AMS in Apple laptops.

This is my first package and I need a sponsor.
Comment 1 Steve Traylen 2009-06-15 15:58:49 EDT
Hi Thomasz,
 
I am not able to sponsor you but I can hopefully offer some comments.

There are some rpmlint errors:

$ rpmlint ../RPMS/x86_64/hdapsd-20090401-1.fc11.x86_64.rpm 
hdapsd.x86_64: W: non-conffile-in-etc /etc/event.d/hdapsd
hdapsd.x86_64: W: non-conffile-in-etc /etc/udev/rules.d/99-hdapsd.rules
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

which to me look like they could and should be fixed.

Maybe add a
%doc README AUTHORS ChangeLog ....

these files never do any harm to be included.
 Steve.
Comment 2 Tomasz Torcz 2009-06-17 05:48:53 EDT
I've uploaded -2 version:
Spec URL: https://pipebreaker.pl/~zdzichu/pkgs/hdapsd.spec
SRPM URL: https://pipebreaker.pl/~zdzichu/pkgs/hdapsd-20090401-2.fc11.src.rpm

which marks upstart & udev files as config files. I've left other as non-marked as doc, because %doc causes them to be installed two times.

Thank you for your comments Steve!
Comment 3 Steve Traylen 2009-06-21 16:02:48 EDT
Hi,
I think the README will have to marked as %doc somehow and not end
up in /usr/share/doc/hdapsd/

I would rm the 

$RPM_BUILD_ROOT/usr/share/doc/hdapsd/README 

file after it was created by the make install and then 
add the README direct from the source tree with

%doc README.

rpmlint works fine.

Something else to do whilst waiting for a package reviewer submit it
as a scratch build to to koji.
Start by getting an account and proceed onto Koji.
http://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account
  Steve
Comment 4 Mamoru TASAKA 2009-07-03 13:44:45 EDT
spec/srpm seems 404...
Comment 5 Steve Traylen 2009-07-10 10:30:32 EDT
Spec and SRPM seem to be back now.
Comment 6 Tomasz Torcz 2009-07-14 12:03:22 EDT
Scratch build completed as http://koji.fedoraproject.org/koji/taskinfo?taskID=1473549 . I'm still thinking about those README, ChangeLog files. They are normally installed into /usr/share/doc/hdapsd. Marking them %doc caused *additional* installation into /usr/share/doc/hdapsd-20090401.
Comment 7 Mamoru TASAKA 2009-07-16 10:18:20 EDT
Well, for -2:

* SourceURL
  - For sourceforge.net hosted URL, follow:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

* $RPM_BUILD_ROOTvs %buildroot
  - Please choose one and don't use both.

* Installing %SOURCE{1,2,3}
---------------------------------------------------
cp %SOURCE1 .
...
...
cp -p hdapsd.event %{buildroot}%{_sysconfdir}/event.d/hdapsd
---------------------------------------------------
  - This can simply be:
---------------------------------------------------
cp -p %SOURCE1 %{buildroot}%{_sysconfdir}/event.d/hdapsd
---------------------------------------------------

* Document installation path
  - By default Fedora installs document files under
   %{_defaultdocdir}/%{name}-%{version}. I suggest to once
   remove all files under %{_defaultdocdir}/%{name} and
   use %doc as:
---------------------------------------------------
%install
rm -rf %{buildroot}
....
make install DESTDIR=%{buildroot}

....
rm -rf %{buildroot}%{_defaultdocdir}/%{name}
....
....
%files
%doc AUTHORS
%doc COPYING
%doc ChangeLog
%doc README
....
--------------------------------------------------
  ! By the way installing "COPYING" file is rather mandatory
    if it exists.
Comment 8 Tomasz Torcz 2009-07-21 15:59:59 EDT
Mamoru,

thank you for your input. I've incorporated changes into:

Spec URL: http://pipebreaker.pl/~zdzichu/pkgs/hdapsd.spec
SRPM URL: http://pipebreaker.pl/~zdzichu/pkgs/hdapsd-20090401-3.fc11.src.rpm
Scratch : http://koji.fedoraproject.org/koji/taskinfo?taskID=1490583
Comment 9 Mamoru TASAKA 2009-07-23 10:02:18 EDT
Well, for -3:

* SourceURL
  - Using %{version} macro in Source0 is preferred as with it
    you probably won't have to change Source0 when the version
    is upgraded:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Using_.25.7Bversion.7D

* Installing %SOURCE{1,2,3}
---------------------------------------------------------
cp %SOURCE1 .
cp %SOURCE2 .
cp %SOURCE3 .
---------------------------------------------------------
  - I think now these lines are not needed.
Comment 10 Mamoru TASAKA 2009-07-23 10:04:24 EDT
And now:

-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on my wiki page:
http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets
(Check "No one is reviewing")

Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
Comment 11 Mamoru TASAKA 2009-08-03 11:07:22 EDT
ping?
Comment 12 Tomasz Torcz 2009-08-03 16:47:02 EDT
I'm still here, sorry for delay. Combination of holiday season and some of work issues made me slacking lately. I haven't pre-reviewied any requests yet, but I'm going to. I'm still interested in becoming part of Fedora.
Comment 13 Mamoru TASAKA 2009-08-24 11:39:57 EDT
ping again?
Comment 14 Tomasz Torcz 2009-08-26 18:11:22 EDT
My comments for my first unofficial review are at
https://bugzilla.redhat.com/show_bug.cgi?id=518799#c4
Comment 15 Mamoru TASAKA 2009-08-27 02:55:35 EDT
Well, okay.

------------------------------------------------------
  This package (hdaspd) is APPROVED by mtasaka
------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji)".

Now I am sponsoring you.

If you want to import this package into Fedora 10/11, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Removing NEEDSPONSOR.
Comment 16 Tomasz Torcz 2009-08-28 16:44:10 EDT
New Package CVS Request
=======================
Package Name: hdapsd
Short Description: Protects hard drives by parking head when fall is detected
Owners: ttorcz
Branches: F-12
InitialCC: ttorcz
Comment 17 Kevin Fenzi 2009-08-28 23:22:33 EDT
cvs done.
Comment 18 Mamoru TASAKA 2009-09-04 10:52:41 EDT
Please rebuild this package on bodhi.
Comment 19 Tomasz Torcz 2009-09-04 12:18:05 EDT
Uh, I've got problem. I did all the steps from wiki/PackageMaintainers/Join, but "make update" fails:

% make update
Creating a new update for  hdapsd-20090401-4.fc12 
Password for ttorcz: 
Creating a new update for  hdapsd-20090401-4.fc12 
hdapsd-20090401-4.fc12 not tagged as an update candidate

Yet it seems that tag already exists:
% make tag
cvs tag  -c hdapsd-20090401-4_fc12

ERROR: Tag hdapsd-20090401-4_fc12 has been already created.
The following tags have been created so far
hdapsd-20090401-4_fc13:devel:ttorcz:1252079380
hdapsd-20090401-4_fc12:F-12:ttorcz:1252079571
cvs tag: Pre-tag check failed
cvs [tag aborted]: correct the above errors first!
make: *** [tag] Error 1


What to do?

BTW, koji build is here: http://koji.fedoraproject.org/koji/buildinfo?buildID=130621
Comment 20 Mamoru TASAKA 2009-09-04 13:00:12 EDT
Currently F-12 koji build will go to rawhide tree directly
so "make update" is not needed.

(By the way if you want to build this for F-11 please see
 https://fedoraproject.org/wiki/CVS_admin_requests
 and write "Package Change Request" here)

Closing as F-12/13 build is done.
Comment 21 Tomasz Torcz 2009-09-04 13:29:55 EDT
I've got my laptop upgrade to rawhide, so I will pass F11. I wouldn't have chance to maintain it properly.
Thank you very much for review and sponsorship!

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