Hello Dean, I've done a bit of re-reviewing the packaging, the items which are I believe to be fixed I have left uncommented, the ones which I think may need some more work are commented in some way: On Sat, 2011-12-17 at 16:40 +0800, Paul Wise wrote: > On Fri, Dec 16, 2011 at 10:37 AM, Dean Evans wrote: > > > I am looking for a review of my packaging efforts for pentobi, a clone > > of the strategy board game Blokus. It is my first project to package and > > it will also be the first time pentobi is in Debian so it will be > > going through the NEW queue. > > Quick review: > > Please fill out the Vcs-* URLs (this is the location of the Debian > packaging). If you don't have it in a VCS yet, take a look at this: > > http://wiki.debian.org/Games/VCS This has been fixed, it would be nice if you pushed your packaging changes continuously to git in order for review though, I automatically started looking at git when you asked for re-review ;) > > The package description could use some reformatting/cleanup. I'd > suggest asking on debian-l10n-english. > > No need for the extra blank line in debian/watch > > The upstream README doesn't look that useful, I would drop it from the > binary package. > > How about adding a longtitle to the debian/menu file? > > If the SVG files in data/ are modified, the PNG files in that same > directory will not be re-rendered from the SVG files. I would suggest > that upstream should drop the PNG files and use either Inksape or rsvg > to create them at build time. This has not changed in the sources I'm assuming you have forwarded this upstream though? > > The Icon field in the pentobi.desktop file should not be an absolute > path, just the name of the icon file, without the extension > (Icon=pentobi). This is so that the user can override any icon and > also so that the deskop can use the SVG file when that is appropriate. > > The regression tests require gogui-regress, it would be nice to > package that too so they could be run during the build. Have you looked into this? Might not happen for this release, but might good to note down as a to-do in Debian? > > application-x-blokus-sgf.svg is public domain, not GPL-3.0+ > > view-fullscreen.png is not copyright Markus Enzenberger but Jakub > Steiner. I would wager that other images in the package are similar. > > go-next.png go-previous.png go-down.png go-first.png go-last.png > go-up.png say they were produced in Inkscape, but there is no SVG > source file available for them. I'd wager go-home.png and lots of > other images were also produced in Inkscape even tho there is no text > saying they were. Similarly board_trigon.png pieces.png > pieces_trigon.png position_trigon.png view-fullscreen.png say they > were made in GIMP but there are no XCF files available. These files > are all allegedly GPL-3.0+, making them not redistributable without > source (probably SVG/XCF). Given that they probably also have a > different copyright holder, I'm not sure what the license actually is. > > It would be nice if upstream could put copyright holders and license > grant headers into each source code file. I see no changes with respect to this, did you look into it? Are many of these from, or derived from, Tango icons? Is that why the license is set as public domain? If that is the case: Where Pentobi have made modifications to them it would be nice to state that and include the (svg) source from the tango theme alongside. Where the icons are completely unmodified, you still need to state the public domain dedication in d/copyright. And likely include the svg equivalents for the sake of DFSG. > > desktop-file-validate warnings: > > pentobi.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated > pentobi.desktop: warning: value "Application;Game;BoardGame;" for key > "Categories" in group "Desktop Entry" contains a deprecated value > "Application" > > dpkg-shlibdeps warning: > > dpkg-shlibdeps: warning: dependency on libboost_thread.so.1.46.1 could > be avoided if "debian/pentobi/usr/games/pentobi > debian/pentobi/usr/games/pentobi-thumbnailer" were not uselessly > linked against it (they use none of its symbols). > > lintian complaints (run lintian-info -t tagname for descriptions): > > P: pentobi source: unversioned-copyright-format-uri > http://dep.debian.net/deps/dep5 > I: pentobi: spelling-error-in-binary usr/games/pentobi Programm Program > I: pentobi: spelling-error-in-binary usr/games/pentobi Programm Program > I: pentobi: spelling-error-in-binary usr/games/pentobi Programms Programs > I: pentobi: spelling-error-in-binary usr/games/pentobi allows to allows one to The Programm spelling is from German, i.e. false positives, the "allows to" -> "allows one to" or "allowing" would still be something to forward upstream though. > P: pentobi: no-upstream-changelog I was thinking you could install upstream NEWS as a changelog, that is it's current use, isn't it? > I: pentobi: description-synopsis-might-not-be-phrased-properly > I: pentobi: desktop-entry-contains-encoding-key > usr/share/applications/pentobi.desktop:2 Encoding > > -- > bye, > pabs > > http://wiki.debian.org/PaulWise > > In addition I have a few more comments: * Installing an xpm 32x32 icon for usage by the debian menu in /usr/share/pixmaps/ would be nice, Imagemagick's convert is convenient for this purpose: convert foo.svg -resize 32x32 foo.xpm (You obviously need to build-depend on imagemagick in that case) * Specify above icon in the menu file * Adding "Format: 1.0" to the .desktop file is, I think, nice in order to follow the specification. I must say Pentobi looks fun :) -- Martin Erik Werner <martinerikwerner@gmail.com>
Attachment:
signature.asc
Description: This is a digitally signed message part