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)
> > * 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.
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/
> 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 :)
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?
> > + #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.
+ remove the duplicated entry. wrap-and-sort from devscript can do
this for you, I think.
* 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.
+ their addition is not documented in d/changelog => blocker.
* the addition of debian/source/format is not documented either
> 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
--
regards,
Mattia Rizzolo
GPG Key: 66AE 2B4A FCCF 3F52 DA18 4D18 4B04 3FCD B944 4540 .''`.
more about me: http://mapreri.org : :' :
Launchpad user: https://launchpad.net/~mapreri `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia `-
signature.asc
Description: PGP signature
