Bug 463400 - (xmmsctrl) Review Request: xmmsctrl - command line control utility for xmms
Review Request: xmmsctrl - command line control utility for xmms
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-23 02:26 EDT by Orcan Ogetbil
Modified: 2008-10-07 05:50 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-09-29 01:53:54 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 Orcan Ogetbil 2008-09-23 02:26:21 EDT
Spec URL: http://6mata.com:8014/xmmsctrl/xmmsctrl.spec
SRPM URL: http://6mata.com:8014/xmmsctrl/xmmsctrl-1.8-2.src.rpm

Description: xmmsctrl is a small utility to control xmms from the command line. Its goal is to be used coupled with sh to test xmms state and perform an appropriate action, e.g. if playing then pause else play. The interest of this is to bind keys in a window manager to have control over xmms with keys that do play/next/pause, prev, control sound...

This is my first package and I am seeking a sponsor.
Comment 1 Mamoru TASAKA 2008-09-27 14:43:18 EDT
Some notes:

* Disttag
  - Please consider to use %{?dist} tag for release number:
    https://fedoraproject.org/wiki/Packaging/DistTag

* ExclusiveArch
  - Would you explain why these ExclusiveArch is needed?

* Optflags
  - Build log shows that Fedora specific compilation flags
    are not honored correctly. As the result debuginfo rpm is
    currently not useful.
    You can check what optflags are used by
    $ rpm --eval %optflags
    ref:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags

* Parallel make
  - Support parallel make if possible:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make
    If this package does not support parallel make, write as such
    in the spec file as comments.

* File permission
  - Usually normal (non-executable) files should have 0644 permission
    (not 0444).

* %defattr
  - We recommend %defattr(-,root,root,-)

* Comment in macros
---------------------------------------------------
#%defattr(0755,root,root)
---------------------------------------------------
  - In comments and %changelog, use %% (instead of %) so that macros
    won't be expanded in comments or %changelog.
Comment 2 Orcan Ogetbil 2008-09-27 18:37:05 EDT
Thanks for the review. 
I fixed everything you said. The ExclusiveArch isn't needed, I took it off. One little question about the optflags: I added $RPM_OPT_FLAGS to the 'make'. Is that enough?

Here are the updated files:
Spec URL: http://6mata.com:8014/xmmsctrl/xmmsctrl.spec
SRPM URL: http://6mata.com:8014/xmmsctrl/xmmsctrl-1.8-3.fc9.src.rpm

Let me know if there's anything else to change.


%changelog
* Sat Sep 27 2008 Orcan Ogetbil <orcanbahri[AT]yahoo[DOT]com> - 1.8-3
- Added DistTag
- Removed ExclusiveArch
- Changed 0444 file permissions to 0644
- Changed %%defattr to (-,root,root,-)
- Honored Fedora specific compilation flags
- Support parallel make
Comment 3 Mamoru TASAKA 2008-09-28 09:32:57 EDT
(In reply to comment #2)
> Thanks for the review. 
> I fixed everything you said. The ExclusiveArch isn't needed, I took it off. One
> little question about the optflags: I added $RPM_OPT_FLAGS to the 'make'. Is
> that enough?

 - I guess fixing (specifying) "WARN" or "CC" rather than "CFLAGS" is simpler, like:
   make %{?_smp_mflags} WARN="${RPM_OPT_FLAGS}"

* Other things are okay.
* New comtributor who wants to get sponsored are requested to either
  - submit another review request
  - or do a pre-review of other person's review request
  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

  For your case you have another review reques (bug 463996), which seems
  in good shape.

---------------------------------------------------------------------
    This package (xmmsctrl) 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 8/9, 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 4 Orcan Ogetbil 2008-09-28 12:10:36 EDT
Thanks again. Here is the final version:

Spec URL: http://6mata.com:8014/xmmsctrl/xmmsctrl.spec
SRPM URL: http://6mata.com:8014/xmmsctrl/xmmsctrl-1.8-4.fc9.src.rpm

It will be CVS'd ASAP.
Comment 5 Orcan Ogetbil 2008-09-28 12:16:12 EDT
New Package CVS Request
=======================
Package Name: xmmsctrl
Short Description: Command line control utility for xmms
Owners: oget
Branches: F-8 F-9
InitialCC: itamar mtasaka
Comment 6 Mamoru TASAKA 2008-09-28 12:34:11 EDT
(In reply to comment #4)
> Spec URL: http://6mata.com:8014/xmmsctrl/xmmsctrl.spec
> SRPM URL: http://6mata.com:8014/xmmsctrl/xmmsctrl-1.8-4.fc9.src.rpm

Okay. Now please wait for the procedure done by CVS admins.
Comment 7 Kevin Fenzi 2008-09-28 15:25:51 EDT
cvs done, except that user 'itamar' doesn't seem to exist in the fedora account system. Can you check that username and reset the fedora-cvs flag if you need me to process any further.
Comment 8 Fedora Update System 2008-09-28 16:44:34 EDT
xmmsctrl-1.8-4.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/xmmsctrl-1.8-4.fc9
Comment 9 Fedora Update System 2008-09-28 16:47:11 EDT
xmmsctrl-1.8-4.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/xmmsctrl-1.8-4.fc8
Comment 10 Mamoru TASAKA 2008-09-29 01:53:54 EDT
Okay, now closing.
Comment 11 Fedora Update System 2008-10-07 05:47:37 EDT
xmmsctrl-1.8-4.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 12 Fedora Update System 2008-10-07 05:50:37 EDT
xmmsctrl-1.8-4.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

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