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

Re: review for inkcut/2.1.6-1



Hi Jeroen,

On 9/13/25 11:51, Jeroen Ploemen wrote:
I took a look at the inkcut package, up for sponsorship in the Python
team:

Thanks for the detailed review.


* copyright: according to the readme that ships in the same dir, the
   famfamfam part of inkcut/res/icons/ is the under cc-by-2.5, not 3.0.

Thanks for catching this. I do not know from where I got the wrong license.

* copyright: missing entries for:
   tests/test_order.py:2:Copyright (c) 2022, Karlis Senko
   tests/test_filters.py:2:Copyright (c) 2022, Karlis Senko
   (thank you very much for correctly listing all the others!)

Thanks again, for spotting this. I consolidated the two files with the other entry for the same author (with some UTF-8 chars in the name, but I verified that it is the same person).

* rules: there appears to be a typo in the pod2man commandd.

Fixed.

* rules: override -> execute_after?

Fixed.

* control: missing (build-)deps on python3-atom, python3-qtpy.
   Some of those are currently pulled in indirectly (for example atom
   via enaml), but they do get imported in inkcut's own code and
   therefore should be direct dependencies.

I added the missing dependencies: python3-atom, python3-qtpy, and python3-cups

* control: no Python deps for inkscape-inkcut, although there's
   Python code in the plugins importing external modules (lxml). Same
   as above, you want to add an explicit dependency.

If added python3-lxml and ${python3:Depends}, but I do not know if the second one is useful. It does not seem to add any dependencies.

   The other import in plugins/inkscape (inkex) appears to be part of
   inkscape itself (which is already a hard dependency), and installed
   into its private directory at /usr/share/inkscape/extensions/inkex/.
   Inkscape-inkcut in turn installs its plugins files into
   /usr/share/inkscape/extensions/inkcut/; I assume inkscape takes
   care of the python path to make these imports work?

Yes, inkscape will take care of it.

* control: could leverage ${source:Extended-Description} and
   ${source:Synopsis} instead of repeating the identical text for
   every binary package, see [1] for numerous examples.

Done.

* d/upstream/metadata: links to the upstream documentation website
   [2] and user forum [3] could be useful additions here.

Added the two links as "Documentation" and "Contact". (or is there anything more appropriate?)

* patches/Fix-missing-device-protocol.patch: does this one originate
   upstream? And if not, should it be forwarded?

This one fixes a warning. I changed the fix and forwarded. It is nothing critical, but I think I fixed it while debugging another error.

* the inkcut binary package is currently designed as a module package
   (with files stored in /usr/lib/python3/dist-packages/), rather than
   as an application (that just happens to be written in Python).

   The latter would typically use a directory in /usr/share/<pkgname>/
   (see 'inkscape' or 'nfoview' for examples) so that the module is
   "private" (as in: not on the global python path). Is the inkcut
   module actually intended to be imported by 3rd party scripts, or
   should the inktcut binary package be changed to an "application
   layout"?

I moved the python package to /usr/share/inkcut, moved the executable (and added a symlink to /usr/bin), and adjusted the config for the autopkgtests.


Thanks,
Alex

Attachment: OpenPGP_0x0BD13B63E2A9AF58.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature


Reply to: