28.07.2013 02:35, Bas Wijnen пишет: > Hi, > > It looks good now, but I found one issue I missed before: network.c and > network.h seem to be missing their copyright header. According to > debian/copyright it should be the same as all other files, but it isn't there. > > Thanks, > Bas > > On Sat, Jul 27, 2013 at 11:01:55PM +0400, Anton Balashov wrote: >> 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 >>> >> > > > Hi, Bas. I fixed the missed copyright header in network.* A new version (1.1.8-3) of package here: http://mentors.debian.net/package/curseofwar Thank you for your help. Could you please sign my public gnupg key? I can show you my docs via any video steam or via any another way you want. Regards, Anton
Attachment:
signature.asc
Description: OpenPGP digital signature