On 12/11/2015 06:43 PM, Mattia Rizzolo wrote: > On Fri, Dec 11, 2015 at 05:19:01PM -0500, Jacob Adams wrote: >> On 12/06/2015 09:05 AM, Mattia Rizzolo wrote: >>> First, please be a bit more patiente with your pings; following up only >>> 5 days later is very much not helpful. >> Sorry about that. I should have realized how soon it was before I pinged. > No worries, just be aware we (in debian, that's general) we're not used > to reply in real time or few hours or 1 day... > >>> On Wed, Nov 25, 2015 at 03:10:28PM -0500, Jacob Adams wrote: >>> * trailing whitespace on debian/control:29 >> Fixed > oops, there is also one in debian/control:25 (sorry) How are you catching these? I don't mean to leave them and I don't mind fixing them, but I can't see trailing whitespace (because it looks like blank space :) ) and so seem to accidentally leave it everywhere. > >>> * please be a lot more verbose on the changelog. "Redo packaging" is >>> not satisfactory at all. Every change should be documented. >> I've updated it to reflect all changes. Should I update the timestamp? >> (It was generated by dch when I first made the changelog). > The timestamp down there is not really relevant, though you might > consider updating it right before uploading (usually I do a dummy commit > before start working on the package, keeping the distribution UNRELEASED > and an empty changelog, and at the end I do `gbp dch --auto -R`, which > also updates the timestamp. Ok I just updated it as it wasn't hard. > > Btw, you added 2 trailing whitespaces at line 5 and 11. > Anyway, the changelog looks nearly good to me now, see below. > >>> * debian/patches/*: if possible a URL of the forwarded patch would be >>> nice >> They've all been forwarded via email, so no urls unfortunately. I did, >> however, have one unnecessary patch in there so I've removed it. I just >> emailed upstream about the two patches I'm still using (The FHS one was >> rejected without any reasoning as it was with a few others and simply >> not applied so I've asked for a reason. The -fPIC one is new). > ok, if they where private email, then 'yes' is totally fine. > >>> * debian/rules: >>> + trailing whitespace at line 11 >>> + with debhelper compat 9 exporting the build flags that way is not >>> needed anymore, so lines 3-4-5 can go away >> Fixed. > cool! I love how cute they look packages with that quasi-empty d/rules! > >>> * you removed the postinst and prerm with the update-alternatives calls, >>> that looks useful to me; why? >> Because I did not look over the previous packaging closely enough :) >> I've added them back. I've also modified them slightly to call set -e in >> order to avoid a lintian warning. > :) > >>> * even if you try to enable the hardening in d/rules, that doesn't work, >>> and blhc still complains (and also lintian) >> I'm not sure what to do about this. Should I just have a patch that >> appends $(shell dpkg-buildflags --get $VAR) for CFLAGS and LDFLAGS? > The problem is that that makefile overwrites CFLAGS from the env. This > is enough to deal with it (it's on top of the other patch of yours): > > --- a/Makefile > +++ b/Makefile > @@ -1,4 +1,4 @@ > -CFLAGS = -DSHAPE -Wall -Werror -fPIC > +CFLAGS := $(CFLAGS) -DSHAPE -Wall -Werror -fPIC > LDLIBS = -lXext -lX11 > BIN = $(DESTDIR)/usr/bin/ > Ok I used that. Lintian still complains however, so I'm not sure what else to do. >> After removing the unecessary lines from d/rules the buildflags appear >> to somewhat work but I've had to add a patch to compile objects with >> -fPIC as otherwise I get > yeah, of course you need -fPIC (google can give you thousands of posts > that explain why it is needed). > >>> * From piuparts: >> All these should be fixed by the above. > yeah, it is. > >>> Please also triage the debian bugs: >>> + #681740 was fixed in 1.2-10 >> Should I just close it? > yes, and also settinging the correct "fixed" value :) > I realize that this might seems mean, but I don't feel ok at sponsoring > a package in debian to a person that doesn't grasp the basis of the > debian infra/tooling/procedures like basic bugs handling in the Debian > tracker, like this; even if it's actually difficult to have such > knowledge *before* starting doing stuff. > > It's hard to verify before uploading the very first package of the > sponsoree, so I'm happy to have these 2 bugs to check this :) Ok I marked it as fixed in 1.2-10 and then closed it. Obviously I need to learn the BTS. The next thing I'll do for Debian is fix a few bugs in other packages to get the hang of it. Any other bits of Debian infrastructure I should learn? I'm relatively familiar with the package tracker and sources. > > Though I am also happy because even if this seems to be your first > package in debian you seems to know something about packaging and about > tools, guess you did packaging work somewhere else? I did some packaging before on a few things for Debian but never got a sponsor. In both cases I never got a response from upstream so I gave up as I felt overwhelmed by all the stuff I had to do. Third time's the charm I guess :) >>> + #349680 not sure what to do, but sice you're becoming the maintainer >>> that's your call. probably the best course of action, given where >>> the links in that bug end, is to just close the bug. >> I've closed it. plan9port and 9wm are two different things. > cool! > > None of the stuff above is a blocker for an upload, so this is mostly > waiting for you to ack, and maybe have a chance of improving the quality > and trying to do things better, even if not required. > > More stuff: > > * you removed debian/9wm.docs in favour of debian/docs which is really > the same thing, but in the second one you have a duplicated entry. > + fix the changelog, there you say only 'Remove unnecessary files' but > you actually renamed it > - this for me is a blocker, because I care about d/changelog telling > me the truth. Sorry about that. I've fixed it. > + remove the duplicated entry. wrap-and-sort from devscript can do > this for you, I think. Done. > * debian/patches/*: > + it would be nice to also have an Author: field; feel free to skip > this item if you don't care, it's just nice and more important for > bigger patches, where you might want to go nagging the original > author, or you have to deal with serious copyright > problems/attribution. I've add Author fields. Should I add you to the Author field of the CFLAGS patch? > + their addition is not documented in d/changelog => blocker. Done. > * the addition of debian/source/format is not documented either > Done. >> Thanks for your help! > You're welcome, and I hope you don't feel demotivated by me being so > picky! Every DD is different and you found one that cares about > trailing whitespaces and thorough changelog :P Honestly, thank you for being so picky. I would much rather take longer and learn how all of this is supposed to work than do it quickly and not understand it.
Attachment:
signature.asc
Description: OpenPGP digital signature