Bug#808538: RFS: corebird

[Iain R. Learmonth]

Hi,

On Wed, Dec 30, 2015 at 11:47:16AM +0800, Paul Wise wrote:
> If the upstream build system supports parallel builds, you are just
> wasting buildd time by removing that option, please put it back in.
> 
> The only reason that it isn't the default yet is because not all
> upstream build systems work when run in parallel mode.

Ok cool. It's not worth doing another upload to add that, but it can be
added in on the next upload of the package.

When I asked before about this, although this was a while ago, I was told
the buildds would only use one core per build anyway so I didn't think this
would be useful. I guess this has changed now.

Thanks,
Iain.

-- 

Bug#808538: RFS: corebird

[Philip Rinn]

Hi Iain,

thanks for the detailed review!

On Thu, 24 Dec 2015 21:37:31 + "Iain R. Learmonth"  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



signature.asc
Description: OpenPGP digital signature

Bug#808538: RFS: corebird

[Paul Wise]

On Wed, Dec 30, 2015 at 2:16 AM, Philip Rinn wrote:

> I removed the "--parallel" now.

If the upstream build system supports parallel builds, you are just
wasting buildd time by removing that option, please put it back in.

The only reason that it isn't the default yet is because not all
upstream build systems work when run in parallel mode.

-- 
bye,
pabs

https://wiki.debian.org/PaulWise

Bug#808538: RFS: corebird

[Iain R. Learmonth]

Hi,

On Thu, Dec 24, 2015 at 09:37:31PM +, Iain R. Learmonth wrote:
>   export DEB_BUILD_MAINT_OPTIONS = hardening=+all
>   export DEB_LDFLAGS_MAINT_APPEND = -Wl,--as-needed

Apologies, I was wrong. These are not enabled by default with compat level
9. As these options may be useful for some packages, I will leave it to you
to decide whether or not you still want these here.

Thanks,
Iain.

-- 

Bug#808538: RFS: corebird/1.1-1 [ITP]

[Iain R. Learmonth]

owner 808538 !
kthxbye

Hi,

Just taking a look at this now.

Thanks,
Iain.

-- 

Bug#808538: RFS: corebird

[Iain R. Learmonth]

Hi,

Unfortunately this is not yet ready for upload. It's nearly there though.

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.

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.

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

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

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.

  export V = 1

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

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.

  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.

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.

Thanks,
Iain.

--

Bug#808538: RFS: corebird

[Mattia Rizzolo]

disclaimer: I did not look at the package at all.

On Thu, Dec 24, 2015 at 09:37:31PM +, Iain R. Learmonth wrote:
>   export V = 1
> 
> I'm not sure what $V does. Is this really necessary for the release in the
> Debian archive?

Some people (me, for instance) like to have the build as verbose as
possible.  locally I have DH_VERBOSE exported in my build env.
For this debhelper started passing a --disable-silent-rules to
autoconf and setting something else for other build systems, for example.
I'd love to see such common variables exported too, and ISTR there is a
bug for something like this.

>   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.

This is going to be the default for debhelper compat 10.  I tend to
force --parallel for my packages when the build takes more than 10
minutes otherwise and in my build env I have
DEB_BUILD_OPTIONS=parallel=4, since that way I have a huge performance
boost.  Also this is nice for buildds (not only buildd.d.o, I'm also
talking about the ones rebuilding the archive, like rb.d.n, where the
throughput is kinda important), so they can waste less time on your
package and use better their CPU.


> As I said above, I prefer defaults be used whenever possible.

Your call :)
I'm just precising some bits.
This kind of stuff is really in the YMMV / YKINMKBYKIOK field :D


-- 
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

Bug#808538: RFS: corebird/1.1-1 [ITP]

[Philip Rinn]

Package: sponsorship-requests
Severity: wishlist

Dear mentors,

I am looking for a sponsor for my package "corebird"

  Package name: corebird
  Version : 1.1-1
  Upstream Author : Timm Bäder 
  URL : http://corebird.baedert.org
  License : GPL-3+
  Section : misc

It builds those binary packages:

  corebird   - Native Gtk+ Twitter client for the Linux desktop

To access further information about this package, please visit the following
URL:

  http://mentors.debian.net/package/corebird


Alternatively, one can download the package with dget using this command:

  dget -x
http://mentors.debian.net/debian/pool/main/c/corebird/corebird_1.1-1.dsc

More information about corebird can be obtained from
http://corebird.baedert.org.


Regards,

Philip Rinn




-- System Information:
Debian Release: stretch/sid
  APT prefers testing
  APT policy: (600, 'testing'), (550, 'unstable'), (500, 'testing-updates')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.2.0-1-amd64 (SMP w/4 CPU cores)
Locale: LANG=de_DE.utf8, LC_CTYPE=de_DE.utf8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)