Hi, Bas. Thanks for your new review on my package. > In debian/copyright, you write "GPL-3.0+". This is not incorrect, but most people write it as GPL-3+. I got it as template from: $ dh_make --copyright gpl3 Anyway, fixed on "GPL-3+" > README seems to be the manual page plus installation instructions. It's useful in the source (because it's in plain text, and contains installation instructions), but I don't think it adds any value to the binary package. It's a part of origin distribution. Let it be :) I fixed rest of issues from you. Install.back file was removed: I want to try do all via Makefile. $ dpkg -c curseofwar_1.1.8-1_amd64.deb drwxr-xr-x root/root 0 2013-07-27 22:53 ./ drwxr-xr-x root/root 0 2013-07-27 22:53 ./usr/ drwxr-xr-x root/root 0 2013-07-27 22:53 ./usr/share/ drwxr-xr-x root/root 0 2013-07-27 22:53 ./usr/share/pixmaps/ -rw-r--r-- root/root 1299 2013-07-27 22:53 ./usr/share/pixmaps/curseofwar-32x32.xpm -rw-r--r-- root/root 467 2013-07-27 22:53 ./usr/share/pixmaps/curseofwar-16x16.xpm drwxr-xr-x root/root 0 2013-07-27 22:53 ./usr/share/doc/ drwxr-xr-x root/root 0 2013-07-27 22:53 ./usr/share/doc/curseofwar/ -rw-r--r-- root/root 160 2013-07-27 22:53 ./usr/share/doc/curseofwar/changelog.Debian.gz -rw-r--r-- root/root 616 2013-07-27 22:49 ./usr/share/doc/curseofwar/changelog.gz -rw-r--r-- root/root 1436 2013-07-27 12:04 ./usr/share/doc/curseofwar/copyright -rw-r--r-- root/root 3200 2013-07-22 14:57 ./usr/share/doc/curseofwar/README.gz drwxr-xr-x root/root 0 2013-07-27 22:53 ./usr/share/menu/ -rw-r--r-- root/root 302 2013-07-22 14:34 ./usr/share/menu/curseofwar drwxr-xr-x root/root 0 2013-07-27 22:53 ./usr/share/man/ drwxr-xr-x root/root 0 2013-07-27 22:53 ./usr/share/man/man6/ -rw-r--r-- root/root 2949 2013-07-27 22:53 ./usr/share/man/man6/curseofwar.6.gz drwxr-xr-x root/root 0 2013-07-27 22:53 ./usr/games/ Looks like the tree is ok. You can find new version (1.1.8-1) of the package here: https://mentors.debian.net/package/curseofwar Thanks a lot for your help. Regards, Anton. 2013/7/27 Bas Wijnen <wijnen@debian.org>: > On Thu, Jul 25, 2013 at 06:40:14PM +0400, Anton Balashov wrote: >> Hello. > > Hi, > >> Can someone review my package until Bas is busy? >> https://mentors.debian.net/package/curseofwar >> I'm not sure that I done everything right. > > Sorry, I thought you were still fixing other things. I have some more remarks > before I can upload it. Note that many of them are just preferences; if you > just say "I like it better this way", that's perfectly fine. I just want you > to be aware of the options. > > in debian/control, you write debhelper (>= 9.0.0). This is not wrong, but > personally I would just write (>= 9), which is the same for practical purposes. > > You depend on dpkg-dev (>= 1.16.1~), I presume for the hardening flags? You > don't need to (and shouldn't) do that; debhelper 9 takes care of the hardening, > so getting the right dependencies is its responibility, not yours. > > You have two commented-out lines which point to a git repository accesible > through git and through a browser. Those lines are meant for providing links > to the packaging of the program. If you use them, they should not be commented > out. However, the links you have there don't work. If you don't use them, > they should be removed completely. Using them is certainly better, if there is > a public place to find the debian packaging. > > In debian/copyright, you write "GPL-3.0+". This is not incorrect, but most > people write it as GPL-3+. This is identical, because in the format > specification is written: >> versions with trailing dot-zeroes are considered to be equivalent to versions >> without (e.g., "2.0.0" is considered equal to "2.0" and "2") > > The manpage (which is very nice!) says at the end of the description "See HOW > TO PLAY for more details." This seems to point to another file (several > manpages do that, which I find really annoying), but actually points to another > section in the manpage. That is normal for a manpage: first a short > description, then the commandline arguments, then more details. I would remove > this line from the end of the short description, to avoid confusion. > > README seems to be the manual page plus installation instructions. It's useful > in the source (because it's in plain text, and contains installation > instructions), but I don't think it adds any value to the binary package. > > In debian/rules, you have a lot of hardening commands which are no longer > required (or useful) with debhelper 9. Since you're depending on that, you can > (and should) remove them. > > Why are you installing the icons mode 664? That suggests that you would want > to make them owned by a special group, which you don't. And if you would, that > would mean that that group would be allowed to change them, which doesn't seem > useful. I think you want 644, like all other icons. > > And if you do, it seems better to install them directly from > curseofwar.install. You are trying to do that as well (but renamed the file so > it isn't used), but there's a problem there: you're installing the directory > "pixmaps" into /usr/share/pixmaps, leading to a directory > /usr/share/pixmaps/pixmaps. Instead, you should either install pixmaps to > /usr/share, or install pixmaps/* to /usr/share/pixmaps/. > > The first line of the install file should be removed; you're installing > directly into debian/curseofwar, so dh_install doesn't need to do that (and in > fact it gives an error because it can't find the file). > > Thanks, > Bas > > > -- > To UNSUBSCRIBE, email to debian-devel-games-request@lists.debian.org > with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org > Archive: [🔎] 20130727015334.GA17460@fmf.nl">http://lists.debian.org/[🔎] 20130727015334.GA17460@fmf.nl >
Attachment:
signature.asc
Description: OpenPGP digital signature