Bug 334941 - Review Request: phasex - Phase Harmonic Advanced Synthesis EXperiment
Summary: Review Request: phasex - Phase Harmonic Advanced Synthesis EXperiment
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-10-16 17:52 UTC by Anthony Green
Modified: 2007-11-30 22:12 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-10-24 04:34:26 UTC
Type: ---
Embargoed:
panemade: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Anthony Green 2007-10-16 17:52:12 UTC
Spec URL: http://people.redhat.com/green/Fedora/phasex.spec
SRPM URL: http://people.redhat.com/green/Fedora/phasex-0.11.1-1.src.rpm
Description: 
PHASEX is an experimental JACK audio / ALSA MIDI softsynth for Linux
with a synth engine built around flexible phase modulation and
flexible oscillator/LFO sourcing.  Modulations include AM, FM, offset
PM, and wave select.  PHASEX comes equipped with a 12db/octave filter
with two distortion curves, a stereo crossover delay and chorus with
phaser, ADSR envelopes for amplifier and filter, realtime audio input
processing capabilities, and more.

Comment 1 Parag AN(पराग) 2007-10-17 06:01:20 UTC
change 
BASE="X-Fedora Application AudioVideo Audio"
to
BASE="AudioVideo Audio"

Reference: http://fedoraproject.org/wiki/PackagingDrafts/DesktopFiles


Comment 2 Parag AN(पराग) 2007-10-17 06:04:50 UTC
and rpmlint complained
phasex.src: W: mixed-use-of-spaces-and-tabs (spaces: line 13, tab: line 1)
The specfile mixes use of spaces and tabs for indentation, which is a
cosmetic annoyance.  Use either spaces or tabs for indentation, not both.
==> use "sed -i -e 's|\t|  |g' phasex.spec"

for desktop file verification you can even check desktop file as
==>desktop-file-validate phasex.desktop 


Comment 3 Anthony Green 2007-10-17 10:11:04 UTC
Thanks.  Updated files here:

Spec URL: http://people.redhat.com/green/Fedora/phasex.spec
SRPM URL: http://people.redhat.com/green/Fedora/phasex-0.11.1-2.src.rpm



Comment 4 Parag AN(पराग) 2007-10-18 03:42:08 UTC
I really don't see any use of installing desktop file twice.  I guess there is
no use of installing desktop file /usr/share/phasex/phasex.desktop

I suggest following patch which will remove extras source1 
--- phasex.spec 2007-10-18 09:00:26.000000000 +0530
+++ phasex-modified.spec        2007-10-18 08:59:56.000000000 +0530
@@ -2,14 +2,13 @@

 Name:  phasex
 Version: 0.11.1
-Release: 2%{?dist}
+Release: 3%{?dist}
 Summary: PHASEX -- Phase Harmonic Advanced Synthesis EXperiment
 Group:  Applications/Multimedia
 License: GPLv2
 URL:  http://sysex.net/phasex/

 Source0: http://sysex.net/phasex/%{name}-%{version}.tar.gz
-Source1: %{name}.desktop
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

 BuildRequires: glibc-devel >= 2.3.0
@@ -56,7 +55,10 @@
 desktop-file-install --vendor %{desktop_vendor} \
   --dir %{buildroot}%{_datadir}/applications \
   `for c in ${BASE} ${XTRA} ; do echo "--add-category $c " ; done` \
-  %{SOURCE1}
+  $RPM_BUILD_ROOT%{_datadir}/phasex/%{name}.desktop
+
+rm $RPM_BUILD_ROOT%{_datadir}/phasex/phasex.desktop
+

 %post
 touch --no-create %{_datadir}/icons/hicolor || :
@@ -87,7 +89,6 @@
 %{_datadir}/phasex/pixmaps/*
 %{_datadir}/phasex/sys-midimaps/*
 %{_datadir}/phasex/sys-patches/*
-%{_datadir}/phasex/phasex.desktop
 %{_datadir}/applications/%{desktop_vendor}-phasex.desktop
 %{_datadir}/icons/hicolor/*/apps/phasex-icon.png


Comment 5 Anthony Green 2007-10-18 12:57:10 UTC
(In reply to comment #4)
> I really don't see any use of installing desktop file twice.  I guess there is
> no use of installing desktop file /usr/share/phasex/phasex.desktop

Thanks.  Here are the new files...

Spec URL: http://people.redhat.com/green/Fedora/phasex.spec
SRPM URL: http://people.redhat.com/green/Fedora/phasex-0.11.1-3.src.rpm

Comment 6 Parag AN(पराग) 2007-10-19 03:07:15 UTC
Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and for RPMs.
+ source files match upstream.
27109ee65fe21bac1530518b436aa8b3  phasex-0.11.1.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc is small so no need of -doc subpackage.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no static libraries.
+ no .pc files are present.
+ no -devel subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ Desktop files installed correctly.
+ file permissions are appropriate.
+ gtk-update-icon-cache scriptlets used.
+ Package phasex-0.11.1-3.fc8 ->
  Requires: alsa-lib >= 0.9.0 glibc >= 2.3.0 gtk2 >= 2.4.0
jack-audio-connection-kit >= 0.99.0 libX11.so.6 libasound.so.2
libasound.so.2(ALSA_0.9) libatk-1.0.so.0 libc.so.6 libc.so.6(GLIBC_2.0)
libc.so.6(GLIBC_2.1) libc.so.6(GLIBC_2.3) libcairo.so.2 libdl.so.2
libgdk-x11-2.0.so.0 libgdk_pixbuf-2.0.so.0 libglib-2.0.so.0 libgmodule-2.0.so.0
libgobject-2.0.so.0 libgtk-x11-2.0.so.0 libjack.so.0 libm.so.6
libm.so.6(GLIBC_2.0) libpango-1.0.so.0 libpangocairo-1.0.so.0 libpthread.so.0
libpthread.so.0(GLIBC_2.0) libpthread.so.0(GLIBC_2.1)
libpthread.so.0(GLIBC_2.3.2) libpthread.so.0(GLIBC_2.3.3) librt.so.1 rtld(GNU_HASH)
+ GUI App.
APPROVED.


Comment 7 Anthony Green 2007-10-19 12:52:03 UTC
New Package CVS Request
=======================
Package Name: phasex
Short Description: PHASEX -- Phase Harmonic Advanced Synthesis EXperiment
Owners: green
Branches: FE-6 F-7 F-8
InitialCC: 
Cvsextras Commits: yes

Comment 8 Kevin Fenzi 2007-10-21 17:20:57 UTC
cvs done.

Comment 9 Parag AN(पराग) 2007-10-24 04:34:26 UTC
closing this review as I can see this is already built for all requested branches.


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