[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#808538: RFS: corebird



Hi Iain,

thanks for the detailed review!

On Thu, 24 Dec 2015 21:37:31 +0000 "Iain R. Learmonth" <irl@debian.org> wrote:
> Hi,
> 
> Unfortunately this is not yet ready for upload. It's nearly there though.

Good to hear :).

> debian/copyright: This is one of the most important things to get right,
> and you have got this right. Do be careful though licensing your packaging
> under different terms to the upstream code. In this case, it's possible to
> take patches you produce and relicense them under GNU GPL v3 (due to the
> "any later version" clause) so they can be integrated upstream, but this
> would not be the case with incompatible licenses. I prefer to always just
> use the license that upstream used.

Hm, good point, I just use GPL-2+ for all my debian/* files. I'll think about it
but keep it for now.

> debian/control: You've set the section to "misc". I would suggest you set
> this to "web" instead. See Debian Policy §2.4 for more detail on sections.

done.

> The extended description also needs to be slightly more extended. Please
> talk about the features this application has.

done.

> debian/rules: You've got some things in here that I don't believe are
> necessary. I like the debian/rules to be as clean as possible, using as many
> of the current defaults as possible. The following statements are redundant
> as you are using compat level 9:
> 
>   export DEB_BUILD_MAINT_OPTIONS = hardening=+all
>   export DEB_LDFLAGS_MAINT_APPEND = -Wl,--as-needed

I also like to use defaults but here, hardening of builds seems to be important
enough to deviate from defaults.

> It is acceptable to leave in the "nocheck" export as long as that's not a
> permanent thing. Please contact upstream and ask them to make the tests
> pass.

I'll contact upstream and try to solve this.

>   export V = 1
> 
> I'm not sure what $V does. Is this really necessary for the release in the
> Debian archive?

As Mattia explained it produces more verbose build logs which helped me to
understand failing builds on buildds in the past.

> You have an "override_dh_auto_configure" section, please look up
> dh-autoreconf as this will handle this for you. Currently this package is
> not policy compliant as "debian/rules clean" will not restore the package to
> its original state. dh-autoreconf handles storing the old versions for you
> and ensuring the package is policy compliant in that respect.

Thanks for catching this, I changed it now.

>   dh $@ --parallel
> 
> Unless you have a good reason for the --parallel, please remove this. If it
> turns out later that the Debian build systems prefer this option, it would
> become a default. As I said above, I prefer defaults be used whenever
> possible.

I removed the "--parallel" now.

> The package bulds fine using cowbuilder, and lintian has no futher
> complaints, if you can fix these issues then I will be happy to sponsor this
> package for you.

I hope I addressed all your suggestions.

I uploaded a revised build to http://mentors.debian.net/package/corebird

Best,
Philip

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: