[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Review of gm-assistant (Game Master Assistant for role-playing games)

I got no feedback for the last version of the gm-assistant package. I hope it's because it's ok. What am I supposed to do in order to have it uploaded into the official archive ?


On 07/04/2012 02:34, Vincent Prat wrote:

The new version (1.1.3-3) of the gm-assistant package is available for another review:


On 29/03/2012 10:39, Vincent Prat wrote:
The two patches are quite specific to Debian. That's why I didn't include them upstream.

As far as wrapping is concerned, I am currently developing on stable and the wrap-and-sort script is not present yet in my version of devscripts. Besides, there is no backport and the dependencies of the testing package are too strong, so that I can't install it.
Is there an alternative ?

The build warnings are totally benign. Is it important to eliminate them ?

For the sdl-mixer dependency, shall I add it directly in the Depends clause with the >= 1.2.12 constraint ?

On 29/03/2012 03:02, Paul Wise wrote:
More review:

  |sdl-mixer-devel (>= 1.2.12) looks weird, no Debian/Ubuntu/etc dev
packages are named -devel so I would suggest dropping that.

I think you need to Build-Depend on qt4-linguist-tools for lupdate, if
libqt4-dev drops its Depends on qt4-linguist-tools then your package
will fail to build from source (FTBFS).

You are missing imagemagick in the Build-Depends (package currently
FTBFS in sbuild/pbuilder/cowbuilder).

Currently imagemagick is unable to build SVG images because it tries
to run the rsvg command (which went away) instead of rsvg-convert.
Please file a bug about that if needed. I would also suggest that you
Build-Depend on librsvg2-bin and use rsvg-convert to convert the SVG
image instead. You will also need imagemagick since rsvg-convert can
only produce PNG images.

Any particular reason you didn't include the two patches upstream?

Please use dh --parallel otherwise you won't use the available CPUs
efficiently when building.

Please manually wrap the debian/menu file.

Please run wrap-and-sort -sa to automatically do the same to
debian/control and other files.

I think the upstream build system should install the
doc/gm-assistant.6, which would mean you would no longer need to do
that manually with the debian/gm-assistant.manpages file.

The full CC-0 "license" needs to be included in the debian/copyright
file. Please replace the 2nd paragraph of the CC-0 License stanza with
the full legal code.

It would be nice if in the upstream COPYRIGHT file you could document
where all the images came from with URLs to the relevant pages on
openclipart. No need to do that in debian/copyright though.

The first two sections of the upstream README.install look like they
should be in a README file instead of README.install.

Build warnings:

* Preparing the compilation... *

/usr/share/qt4/mkspecs/features/default_post.prf(5):Function 'system'
is not implemented
/tmp/buildd/gm-assistant-1.1.2/sources/engine/Tree.h:117: circular
inclusion of /tmp/buildd/gm-assistant-1.1.2/sources/engine/Branch.h
/usr/include/qt4/QtCore/qstringbuilder.h:45: circular inclusion of
/usr/include/qt4/QtGui/qwmatrix.h:45: circular inclusion of
/usr/include/qt4/QtGui/qactiongroup.h:45: circular inclusion of
/usr/include/qt4/QtGui/qboxlayout.h:45: circular inclusion of
/usr/include/qt4/QtGui/qgridlayout.h:45: circular inclusion of
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.2/translations/gmassistant_fr.qm'...
     Generated 128 translation(s) (128 finished and 0 unfinished)

sources/engine/Item.cpp:52:6: warning: unused parameter 'root'
sources/engine/Item.cpp:56:6: warning: unused parameter 'root'
sources/engine/Item.cpp: In static member function 'static std::string
sources/engine/Item.cpp:69:1: warning: control reaches end of non-void
function [-Wreturn-type]
sources/engine/SoundEngine.cpp:134:6: warning: unused parameter
'channel' [-Wunused-parameter]
sources/engine/Tree.cpp:522:16: warning: unused parameter 'i'
sources/widgets/QCustomTreeWidget.cpp:156:6: warning: unused parameter
'e' [-Wunused-parameter]
sources/windows/PictureWindow.cpp:48:6: warning: unused parameter 'e'

Reply to: