Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.maint.python > #14549 > unrolled thread

Review of Debian package lazy-loader

Back to article view | Back to linux.debian.maint.python

Contents

  Review of Debian package lazy-loader Louis-Philippe Véronneau <pollo@riseup.net> - 2022-10-22 06:20 +0200
    Re: Review of Debian package lazy-loader Bo YU <tsu.yubo@gmail.com> - 2022-10-22 14:10 +0200

#14549 — Review of Debian package lazy-loader

Hello,

This is my review of the lazy-loader package you asked the Debian Python 
Team to sponsor in the Debian archive.

1. In d/control, I'm not sure to understand why the binary package is 
marked as "Multi-Arch: foreign", as this package isn't arch dependent?

2. In d/control, your list of build-dependencies for the source package 
should be reviewed.

* Why do you need 'python3-setuptools' when this package uses flit?
* Why the dependency on 'python3-toml'?
* You should build-depend on 'pybuild-plugin-pyproject', as this is a 
PEP517 compatible package. The 'old' flit plugin for pybuild is being 
deprecated in favor of this one.
* since 'python3-pytest' is only used for tests, it should be marked as 
so using <!nocheck>

3. In d/control, although Stefan van der Walt is the author, the 
copyright should only be "Scientific Python project", as that's the only 
licensing information in this package.

4. There are many commented lines in d/rules that should just be 
removed. Note that you do not need 'export DH_VERBOSE = 1' nor 'export 
PYBUILD_SYSTEM=flit' (since you'll now build with 
'pybuild-plugin-pyproject')

5. The autopkgtests you are running in d/tests are redundant. autodep8 
already does this for python.

You should instead run the upstream test suite as autopkgtests: they are 
much more meaningful.

Have a look at this example:

https://salsa.debian.org/python-team/packages/metalfinder/-/tree/debian/master/debian/tests

6. In d/changelog, you marked your entry as "unstable", whereas it 
should be UNRELEASED. Please re-read the DPT's policy with regards to this.

7. Although I have not listed them here, pretty much all of the lintian 
tags raised are relevant errors that you should fix.

----------------------------------

You're 90% there!

I've removed your package from the sponsor queue for now, but feel free 
to re-add it when you feel like you've dealt with my review. I'll be 
happy to sponsor it then.

Thanks for your contribution to Debian.

-- 
   ⢀⣴⠾⠻⢶⣦⠀
   ⣾⠁⢠⠒⠀⣿⡁  Louis-Philippe Véronneau
   ⢿⡄⠘⠷⠚⠋   pollo@debian.org / veronneau.org
   ⠈⠳⣄

[toc] | [next] | [standalone]

#14550

[Multipart message — attachments visible in raw view] — view raw

Hi,
On Fri, Oct 21, 2022 at 11:58:38PM -0400, Louis-Philippe Véronneau wrote:
>Hello,
>
>This is my review of the lazy-loader package you asked the Debian 
>Python Team to sponsor in the Debian archive.
>
>1. In d/control, I'm not sure to understand why the binary package is 
>marked as "Multi-Arch: foreign", as this package isn't arch dependent?

The binary package is arch dependent as it was marked as "Architecture: all"
I think. There is more knowledge for me to here:
https://wiki.debian.org/MultiArch/Hints#set_Multi-Arch:_foreign

Fixed it.

>
>

[...]
>You should instead run the upstream test suite as autopkgtests: they 
>are much more meaningful.
>
>Have a look at this example:
>
>https://salsa.debian.org/python-team/packages/metalfinder/-/tree/debian/master/debian/tests
>

Ok, The package now is updated according to the all above review comments.

>6. In d/changelog, you marked your entry as "unstable", whereas it 
>should be UNRELEASED. Please re-read the DPT's policy with regards to 
>this.

Ok, this is different entry with previous package that has been
sponsored by others DD. But I think 'UNRELEASED' entry is right:)
>
>7. Although I have not listed them here, pretty much all of the 
>lintian tags raised are relevant errors that you should fix.

Yeah, I run lintian the package this time and it got nothing from my
chroot build. The only error from mentor is:
```
Package uploaded for the UNRELEASED distribution
````
I think it should be ok this time.

>
>----------------------------------
>
>You're 90% there!
>
>I've removed your package from the sponsor queue for now, but feel 
>free to re-add it when you feel like you've dealt with my review. I'll 
>be happy to sponsor it then.

Thanks you very much! I have updated it from your valueable review.
Please let me know if there is any issues.

-- 
Regards,
--
   Bo YU

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.maint.python

csiph-web