Re: Review of gm-assistant
On Thu, Mar 22, 2012 at 8:11 PM, Vincent Prat wrote:
> Sorry...
No need to apologise!
> The source package is already on mentors.debian.net :
> http://mentors.debian.net/package/gm-assistant
Quick review:
Since you are upstream, please take a look at these pages:
http://wiki.debian.org/UpstreamGuide
http://www.freedesktop.org/wiki/Games/Upstream
The manual page doesn't need all the comments, please drop lines like this:
.\"
Please get the manual page included upstream if it isn't already.
If you want the package to be maintained in the games team, you should
put the team in Maintainer and yourself in Uploaders.
The Vcs-* fields are commented out and incorrect.
You might want to run wrap-and-sort -sa so that diffs of
debian/control and other files are more readable. You might want to
also manually wrap debian/menu.
Please add a longtitle to debian/menu.
None of the comments in debian/rules are needed, you can remove them
all. You might want to keep the DH_VERBOSE line as a reminder though.
The last paragraph of the package description can be removed since it
is irrelevant to most users. Once the package reaches Debian, debtags
are a good replacement for that.
Please contact upstream and ask them to push tags for releases, that
will enable you to use the watch file below. Also your watch file
doesn't need the first comment line.
version=3
https://github.com/ViviCoder/GM-Assistant/tags .*tarball/([\d\.]+)
data/images/GMA.png and the associated GMA.svg use the non-free
CC-BY-NC-SA 3.0 license. You will need to change the license on that
and get that fixed upstream if you want it in Debian. Also
debian/copright needs to document the license of those files. You may
want to look at the new format for debian/copyright:
http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
Most of the files in data/sound are from
http://www.universal-soundbank.com and that website does not appear to
have any license information, so you (and github/mentors/alioth) are
probably illegally redistributing them. Where are pan.mp3 and
pluie.mp3 from? Who are the copyright holders and what is the license?
What is the source, how were they created?
The upstream README doesn't add anything useful to the binary package
(yet), please don't install it there (yet).
Should you add more information to the upstream README such that it
provides information useful to people installing the binary package on
Debian, please split out the installation and build instructions into
README.install. If you want the full current README to be displayed on
github, you can rename it to README.md.
You might want to bump the debhelper compat to version 9 since that
brings some useful improvements.
Please include the patches upstream.
I would suggest that data/images/GMA.png and data/images/GMA.xpm
should be created at build time since data/images/GMA.svg is the
source for them.
The other files in data/images/ seem to be created by Photoshop, is
there any particular reason you didn't include the Photoshop source
files?
Some warnings from the build process:
/usr/share/qt4/mkspecs/features/default_post.prf(5):Function 'system'
is not implemented
/tmp/buildd/gm-assistant-1.1.1/sources/engine/Tree.h:117: circular
inclusion of /tmp/buildd/gm-assistant-1.1.1/sources/engine/Branch.h
/usr/include/qt4/QtCore/qstringbuilder.h:45: circular inclusion of
/usr/include/qt4/QtCore/qstring.h
/usr/include/qt4/QtGui/qwmatrix.h:45: circular inclusion of
/usr/include/qt4/QtGui/qmatrix.h
/usr/include/qt4/QtGui/qactiongroup.h:45: circular inclusion of
/usr/include/qt4/QtGui/qaction.h
/usr/include/qt4/QtGui/qboxlayout.h:45: circular inclusion of
/usr/include/qt4/QtGui/qlayout.h
/usr/include/qt4/QtGui/qgridlayout.h:45: circular inclusion of
/usr/include/qt4/QtGui/qlayout.h
Updating 'translations/gmassistant_fr.ts'...
Found 128 source text(s) (0 new and 128 already existing)
/usr/share/qt4/mkspecs/features/default_post.prf(5):Function 'system'
is not implemented
Updating '/tmp/buildd/gm-assistant-1.1.1/translations/gmassistant_fr.qm'...
Generated 128 translation(s) (128 finished and 0 unfinished)
sources/engine/Item.cpp:52:6: warning: unused parameter 'root'
[-Wunused-parameter]
sources/engine/Item.cpp:56:6: warning: unused parameter 'root'
[-Wunused-parameter]
sources/engine/Item.cpp: In static member function 'static std::string
Item::stateToStr(Item::State)':
sources/engine/Item.cpp:69:1: warning: control reaches end of non-void
function [-Wreturn-type]
sources/windows/PictureWindow.cpp:48:6: warning: unused parameter 'e'
[-Wunused-parameter]
dpkg-shlibdeps warnings (probably Qt's fault):
dpkg-shlibdeps: warning: package could avoid a useless dependency if
debian/gm-assistant/usr/games/gm-assistant was not linked against
libsigc-2.0.so.0 (it uses none of the library's symbols).
dpkg-shlibdeps: warning: package could avoid a useless dependency if
debian/gm-assistant/usr/games/gm-assistant was not linked against
libglib-2.0.so.0 (it uses none of the library's symbols).
dpkg-shlibdeps: warning: package could avoid a useless dependency if
debian/gm-assistant/usr/games/gm-assistant was not linked against
librt.so.1 (it uses none of the library's symbols).
dpkg-shlibdeps: warning: package could avoid a useless dependency if
debian/gm-assistant/usr/games/gm-assistant was not linked against
libxml2.so.2 (it uses none of the library's symbols).
dpkg-shlibdeps: warning: package could avoid a useless dependency if
debian/gm-assistant/usr/games/gm-assistant was not linked against
libgobject-2.0.so.0 (it uses none of the library's symbols).
dpkg-shlibdeps: warning: package could avoid a useless dependency if
debian/gm-assistant/usr/games/gm-assistant was not linked against
libgthread-2.0.so.0 (it uses none of the library's symbols).
Some warnings from mp3check:
./data/sound/cri.mp3:
1024 bytes of junk before first frame header
invalid id3 tag trailer v1.0 found
anomaly: bitrate 256kbit/s
anomaly: mode stereo
anomaly: no crc
./data/sound/grince.mp3:
222 bytes of junk before first frame header
anomaly: sampling rate 48.0kHz
anomaly: bitrate 192kbit/s
anomaly: mode stereo
./data/sound/pan.mp3:
anomaly: bitrate 192kbit/s
./data/sound/pluie.mp3:
valid id3 tag trailer v1.0 found
frame 1/ 0:00: bitrate switching (128 -> 192)
frame 1673/ 0:43: bitrate switching (192 -> 32)
anomaly: no crc
./data/sound/ak47.mp3:
1024 bytes of junk before first frame header
invalid id3 tag trailer v1.0 found
anomaly: bitrate 192kbit/s
anomaly: mode stereo
anomaly: no crc
--
5 files checked, 4 erroneous files found
(5 anomalies found)
--
bye,
pabs
http://wiki.debian.org/PaulWise
Reply to: