Bug 490317 - Review Request: rumor - Really Unintelligent Music transcriptOR
Review Request: rumor - Really Unintelligent Music transcriptOR
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christian Krause
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 490318
  Show dependency treegraph
 
Reported: 2009-03-15 04:44 EDT by Orcan Ogetbil
Modified: 2009-04-15 14:00 EDT (History)
3 users (show)

See Also:
Fixed In Version: 1.0.3b-2.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-04-15 14:00:43 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
chkr: fedora‑review+
dennis: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Orcan Ogetbil 2009-03-15 04:44:09 EDT
Spec URL: http://oget.fedorapeople.org/review/rumor.spec
SRPM URL: http://oget.fedorapeople.org/review/rumor-1.0.3b-1.fc10.src.rpm
Description: 
Rumor is a realtime monophonic (with chords) MIDI keyboard to Lilypond
converter. It receives MIDI events, quantizes them according to its metronome
on the fly and outputs handwritten-like corresponding Lilypond notation. Tempo,
meter, key and other parameters can be set via command-line options.

rpmlint is silent

Koji F-11 build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1241995
Comment 1 Christian Krause 2009-03-20 19:03:58 EDT
Hi Orcan,

here is my official review, there are only 2 very minor issues:

* rpmlint: TODO
rpmlint SPECS/rumor.spec RPMS/i386/rumor-* SRPMS/rumor-1.0.3b-1.fc10.src.rpm
SPECS/rumor.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 12, tab: line 1)
rumor.src: W: mixed-use-of-spaces-and-tabs (spaces: line 12, tab: line 1)
3 packages and 1 specfiles checked; 0 errors, 2 warnings.
- just a very minor mixture of spaces and tabs

* spec file name: OK

* Source0: OK
- spectool -g works
- URL ok
- sources matches upstream: md5sum
c95917356659b77bb83505cb2512ad1b  rumor-1.0.3b.tar.bz2

* package name: OK, matches upstream

* license: OK
- GPLv2 is acceptable
- rumor.cc explicitely states that "version 2 as published by the Free Software Foundation" should be used
- license field matches actual license
- COPYING packaged

* spec file legible and in American English: OK

* package compiles: OK
- mock
- koji in F9, F10 and F11:
https://koji.fedoraproject.org/koji/taskinfo?taskID=1251851
https://koji.fedoraproject.org/koji/taskinfo?taskID=1251856
https://koji.fedoraproject.org/koji/taskinfo?taskID=1251861

* BuildRequires: OK

* locale handling: OK (no locale available)

* shared libs handling: OK (no shared libs)

* directory ownership: OK
- no directories created

* debuginfo: OK
- seems to be complete, debuginfo available in gdb for rumor

* files not listed twice: OK

* optflags: TODO
- I would delete the -O0 in the configure file completely and not substitute it with -O2. If Fedora changes the rpmoptflags, there will be two different -Ox options (the one from rpmoptflags and the one from the configure) (e.g. "sed -i 's|-O0||' configure" could be used)
- the only uncommon gcc option is "-ggdb3", but that's no problem, it will only add some more info to the debug infos (e.g. macros) - gdb works still fine with rumor's debuginfo

* %clean and rm -rf in %install: OK

* file perms: OK

* %defattr used: OK

* macro usage: OK

* code vs. content: OK
- only code

* large docu in sub-package: OK (n/a)

* *.la files, header, static/dynamic libs, pkgconfig: OK (n/a)

* GUI application needs desktop file: OK (not a GUI app)

* file names UTF8: OK

* basic features: OK
- I've used rumor together with frescobaldi to record some notes from a keyboard attached via MIDI. It worked without any problems.

* scriptlets: OK
- texinfo: Requires ok
- scriptlets for texinfo files correct
- /usr/share/info/dir not part of the package

Best regards,
Christian
Comment 2 Orcan Ogetbil 2009-03-21 03:12:54 EDT
(In reply to comment #1)
 
> * rpmlint: TODO
> rpmlint SPECS/rumor.spec RPMS/i386/rumor-* SRPMS/rumor-1.0.3b-1.fc10.src.rpm
> SPECS/rumor.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 12, tab: line
> 1)
> rumor.src: W: mixed-use-of-spaces-and-tabs (spaces: line 12, tab: line 1)
> 3 packages and 1 specfiles checked; 0 errors, 2 warnings.
> - just a very minor mixture of spaces and tabs
> 

I wonder how this rpmlint survived all my checks. Fixed!


> * optflags: TODO
> - I would delete the -O0 in the configure file completely and not substitute it
> with -O2. If Fedora changes the rpmoptflags, there will be two different -Ox
> options (the one from rpmoptflags and the one from the configure) (e.g. "sed -i
> 's|-O0||' configure" could be used)
> - the only uncommon gcc option is "-ggdb3", but that's no problem, it will only
> add some more info to the debug infos (e.g. macros) - gdb works still fine with
> rumor's debuginfo
> 

Good point. Thanks for bringing this up. 

Spec URL: http://oget.fedorapeople.org/review/rumor.spec
SRPM URL: http://oget.fedorapeople.org/review/rumor-1.0.3b-2.fc10.src.rpm

Thank you Christian, once more :)
Comment 3 Christian Krause 2009-03-31 17:50:05 EDT
all mentioned issues are addressed in the new package:

- compiler optimization flags completely removed, so that only the ones from the rpmoptflags are used:
g++ -DLOCALEDIR=\"/usr/share/locale\" -DHAVE_CONFIG_H -I. -I. -I..     -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -D_REENTRANT -ggdb3  -Wall -pedantic -MT options.o -MD -MP -MF ".deps/options.Tpo" \
          -c -o options.o

- mixes spaces/tabs in spec file fixed:
rpmlint SPECS/rumor.spec RPMS/i386/rumor-* SRPMS/rumor-1.0.3b-1.fc10.src.rpm 
5 packages and 1 specfiles checked; 0 errors, 0 warnings.


APPROVED.
Comment 4 Orcan Ogetbil 2009-03-31 23:01:38 EDT
Thanks a lot!

New Package CVS Request
=======================
Package Name: rumor
Short Description: Really Unintelligent Music transcriptOR
Owners: oget
Branches: F-9 F-10
InitialCC:
Comment 5 Dennis Gilmore 2009-04-01 12:32:30 EDT
CVS Done
Comment 6 Fedora Update System 2009-04-01 13:05:40 EDT
rumor-1.0.3b-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/rumor-1.0.3b-2.fc10
Comment 7 Fedora Update System 2009-04-02 13:11:35 EDT
rumor-1.0.3b-2.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update rumor'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-3182
Comment 8 Fedora Update System 2009-04-15 14:00:38 EDT
rumor-1.0.3b-2.fc10 has been pushed to the Fedora 10 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.