Bug 463400 (xmmsctrl) - Review Request: xmmsctrl - command line control utility for xmms
Summary: Review Request: xmmsctrl - command line control utility for xmms
Keywords:
Status: CLOSED NEXTRELEASE
Alias: xmmsctrl
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-09-23 06:26 UTC by Orcan Ogetbil
Modified: 2008-10-07 09:50 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-09-29 05:53:54 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Orcan Ogetbil 2008-09-23 06:26:21 UTC
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 18:43:18 UTC
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 22:37:05 UTC
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 13:32:57 UTC
(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 16:10:36 UTC
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 16:16:12 UTC
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 16:34:11 UTC
(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 19:25:51 UTC
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 20:44:34 UTC
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 20:47:11 UTC
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 05:53:54 UTC
Okay, now closing.

Comment 11 Fedora Update System 2008-10-07 09:47:37 UTC
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 09:50:37 UTC
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.