Spec URL: http://chkr.fedorapeople.org/review/pianobooster.spec SRPM URL: http://chkr.fedorapeople.org/review/pianobooster-0.6.2-2.fc10.src.rpm Description: A MIDI file player/game that displays the musical notes AND teaches you how to play the piano. PianoBooster is a fun way of playing along with a musical accompaniment and at the same time learning the basics of reading musical notation. The difference between playing along to a CD or a standard MIDI file, is that PianoBooster listens and reacts to what you are playing on a MIDI keyboard. - rpmlint SPECS/pianobooster.spec SRPMS/pianobooster-0.6.2-2.fc10.src.rpm RPMS/i386/pianobooster-0.6.2-2.fc10.i386.rpm RPMS/i386/pianobooster-debuginfo-0.6.2-2.fc10.i386.rpm 3 packages and 1 specfiles checked; 0 errors, 0 warnings. - koji builds: F10: https://koji.fedoraproject.org/koji/taskinfo?taskID=1285986 F9:https://koji.fedoraproject.org/koji/taskinfo?taskID=1285981 I could not build the package for rawhide today. It looks like that lots of other packages fail in F11 right now, too. The package built successfully in rawhide yesterday: F11: https://koji.fedoraproject.org/koji/taskinfo?taskID=1284073 - please note that the packages built in koji can't be used directly in F10 or F9 since there is an incompatible update of qt4 in updates-testing which has the build-f10-overrides tag set
Here are my notes for this package (* need definite attention. ! are suggestions): * We need to package rtmidi seperately and link to it. I already packaged rtaudio before. Things should be similar (you can take rtaudio SPEC file as your starting point) Note that rtmidi is MIT, but it is compatible with GPL. ! Please make use of the %{name} macro. ! I find it better to supply the .desktop file separately, to preserve the original creation date. But that's a matter of taste. ! I prefer using sed+iconv instead of dos2unix to save a BR. But then again this is a matter of taste. If you are going to use dos2unix, could you use the -k flags to preserve timestamps? * Similarly, please use the -p flag with install to preserve the timestamps (of the .png files in this case) ! Please add a GenericName to the .desktop file. Also AudioVideo needs to be added to the Category key, if you want this application to appear in Multimedia group. * There is a typo on the installation of the 64x64 icon. (32x32 should be 64x64) * Parallel make must be supported whenever possible. If it is not supported, this should be noted in the SPEC file as a comment. ! Summary seemed too long to me. It can be just something like "Piano Teacher" ! I think the comma should be removed from the end of this line (for proper English): "The difference between playing along to a CD or a standard MIDI file,"
I've found license problem in this package: The source (gplv3+) contains 3 files ("rtmidi") which are licensed under a modified MIT license. In general this would be ok, but this license contains the statement that modifications must be sent back to the author. I'll let this bug block FE-LEGAL to get their advice. Here is the complete license: RtMidi WWW site: http://music.mcgill.ca/~gary/rtmidi/ RtMidi: realtime MIDI i/o C++ classes Copyright (c) 2003-2009 Gary P. Scavone Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. Any person wishing to distribute modifications to the Software is requested to send the modifications to the original developer so that they can be incorporated into the canonical version. THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
(In reply to comment #1) > Here are my notes for this package (* need definite attention. ! are > suggestions): Thank you for the review. > * We need to package rtmidi seperately and link to it. I already packaged > rtaudio before. Things should be similar (you can take rtaudio SPEC file as > your starting point) I've checked the upstream rtmidi package ( http://www.music.mcgill.ca/~gary/rtmidi/ ) but unfortunately the package doesn't provide any Makefiles so that it is not easily possible to build a library out of the box. Since other projects also use rtmidi just by adding the 3 files, I would vote for not creating an extra library for rtmidi. Once rtmidi's upstream will provide a full-featured package to build dynamic (or static) libs this can be re-considered. > ! Please make use of the %{name} macro. Done. Besides the URL tag the specfile was changed to use the %{name} tag. > ! I find it better to supply the .desktop file separately, to preserve the > original creation date. But that's a matter of taste. Done. I've added the desktop file as separate source file. > ! I prefer using sed+iconv instead of dos2unix to save a BR. But then again > this is a matter of taste. If you are going to use dos2unix, could you use the > -k flags to preserve timestamps? Done. (Using sed now. However I haven't found an option to preserve the timestamp when using sed...) > * Similarly, please use the -p flag with install to preserve the timestamps (of > the .png files in this case) Done. > ! Please add a GenericName to the .desktop file. Also AudioVideo needs to be > added to the Category key, if you want this application to appear in Multimedia > group. Done. I've used your suggested "Piano Teacher" as GenericName and added the AudioVideo category. > * There is a typo on the installation of the 64x64 icon. (32x32 should be > 64x64) Done. > * Parallel make must be supported whenever possible. If it is not supported, > this should be noted in the SPEC file as a comment. Done. Changed to parallel build. I've seen no problems so far (in local as well as in koji/mock builds). > ! Summary seemed too long to me. It can be just something like "Piano Teacher" I've checked other specfiles too and I've seen a bunch of packages with similar summaries. I would rather leave the summary as it is. However, I've re-used the suggestion as the GenericName in the .desktop file. ;-) > ! I think the comma should be removed from the end of this line (for proper > English): > "The difference between playing along to a CD or a standard MIDI file," Done. The modified packages can be found here: Spec URL: http://chkr.fedorapeople.org/review/pianobooster.spec SRPM URL: http://chkr.fedorapeople.org/review/pianobooster-0.6.2-3.fc10.src.rpm
Thanks for the update. (In reply to comment #3) > (In reply to comment #1) > > > ! I prefer using sed+iconv instead of dos2unix to save a BR. But then again > > this is a matter of taste. If you are going to use dos2unix, could you use the > > -k flags to preserve timestamps? > > Done. (Using sed now. However I haven't found an option to preserve the > timestamp when using sed...) > The standard way of making this is: sed -e 's|\r||g' file > file.tmp touch -r file file.tmp mv file.tmp file Let's wait for FE Legal's resolution for the license issue.
The additional clause in the MIT license is a request, not a requirement. Accordingly, it is not a problem. Since the MIT code is compiled together with GPLv3+ code, you do not need to mention it in the License tag (the GPLv3+ terms are more restrictive), unless you wish to do so. License: GPLv3+ is fine. Lifting FE-Legal.
OK, since we don't have a legal problem we can approve this package: ----------------------------------------------- This package (pianobooster) is APPROVED by oget -----------------------------------------------
New Package CVS Request ======================= Package Name: pianobooster Short Description: A MIDI file player that teaches you how to play the piano Owners: chkr Branches: F10 InitialCC:
I assume you want a F-11 branch as well here. cvs done with an F-11 branch too.
pianobooster-0.6.2-4.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/pianobooster-0.6.2-4.fc10
pianobooster-0.6.2-4.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/pianobooster-0.6.2-4.fc11
pianobooster-0.6.2-4.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
pianobooster-0.6.2-4.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.