Hi Peter,
On 12/15/20 8:54 AM, Peter Ji wrote:
> Thank you very much for such a careful review. The information you mentioned
> was very helpful to new comer like me. Especially the list in the end.
> Thanks!
Glad to hear :)
> I tried to repacking dhcpdump as suggested, such as detailed changelog
> and modified other d/* files in the latest upload.
> Although it may still have errors, I will continue to work on.
Alright, let's do another pass then.
d/changelog:
- The update_befor_adoption patch has a spelling mistake
- d/control Multi-arch field is not documented
- On the block "Update the description of dhcpdump", why is there a
sub item? Shouldn't it be in the same sentence?
- In the same block, you are missing a space before the bug info
d/control: looks good to me
d/copyright:
- The first block does not need to enumerate all files. The wildcard
'*' can be use instead. All following entries will override that.
- The license regarding the first block and `debian/*` is incorrect.
Only strsep.c uses the BSD-4-clause-UC.
d/rules:
- You should use DEB_CFLAGS_MAINT_APPEND and DEB_LDFLAGS_MAINT_APPEND
instead of re-defining the flag yourself. See `man dpkg-buildflags`
and `man debhelper` for more info.
- Most of the flag you use are already set by debhelper. You can play
with those by adding a _temporary_ echo $(CFLAGS) in the
debian/rules.
In your case, you should need only the -DHAVE_STRSEP (the rest is
either already defined or unecessary).
- You have an extra space after DEB_CFLAGS_MAINT_APPEND
- In debian/rules and in the Makefile, make and $(CC) are used. Since
there are no ./configure script, cross building the package will
fail trying to use make and cc instead of the correct targeted arch
tool. To workaround that, you should:
- Include /usr/share/dpkg/buildtools.mk at the begining of d/rules.
That will correctly define MAKE and CC variables
- Replace make by $(MAKE) and add the CC variable on the make
command line.
patches/000...:
- The patch description does not describe precisely the changeset.
- The patch description has a spelling mistake
- Remove the <> char from the description. Those are used only in the
email address.
patches/001...:
- You skip the install rule in debian/rules. Therefor, this patch is
not needed. Don't forget to remove the entry from d/changelog as
well.
patches/002...:
- You have a extra space at the end of the line 37.
- Remove the <> char from the description
> I know Debian's Gitlab But not familiar. I will try to apply for access when necessary.
Just so you know, it's open for anyone to register. You just need to
create an account and that it.
--
Baptiste Beauplat - lyknode
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature