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

Bug#1007025: git-multimail 1.6.0 package review



Hi,
On Thu, Sep 15, 2022 at 05:32:33PM +0100, Antoine Beaupré wrote:
Hi!

I've done a quick review of the 1.6.0 package on salsa as of commit
d5bd184a1cf73b752f80dea46d8080493a5e663b.

It looks like there's some leftover stuff in debian/copyright, i would
remove this:

modified   debian/copyright
@@ -2,8 +2,6 @@ Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
Upstream-Name: git-multimail
Upstream-Contact: Matthieu Moy <matthieu.moy@univ-lyon1.fr>
Source: https://github.com/git-multimail/git-multimail
-#
-# Please double check copyright with the licensecheck(1) command.

Files:     *
Copyright: 2014-2022, Matthieu Moy <matthieu.moy@univ-lyon1.fr>

Ok, removed comment here.


Also, I didn't quite follow the work on the test cases, but why did you
replace pep8 by pycodestyle in the patch in debian/patches? The patch
itself doesn't actually explain the *why* (it explains the "what" but we
typically want more than this...)

This time i add dep3 header for the patch. It should be noted that
despite this patch, it is still not helpful for upstream test cases.
I have forwarded this for upstream:
https://github.com/git-multimail/git-multimail/issues/221#issuecomment-1245009306
(To avoid bring noisy for upstream, i just recorded it in a issue)


It seems like you have README.rst both in debian/rules and
debian/docs. Either one of those should be sufficient, and you should
remove the other. Same with the launcher in
python3-git-multimail.install vs debian/rules.

There should be no duplicate file. I have droped they.

I'm also surprised we need that launcher at all. Normally, the
`setup.py` wrapper has a scripts= stanza which should install the
upstream one, why do we do it differently?

yeah. The reason why I use launcher here is bacause that @jcfp helped
me to review it in the previous time:

```
the (large) git_multimail.py file is installed twice, both as a
public module under /usr/lib/python3 and as a script in /usr/bin;
the latter could be replaced by a tiny launcher (take a look at the
nfoview package if you need inspiration; your launcher would be even
shorter because it doesn't need the sys.path manipulation)
```
I am not sure if I understand jcfp's meaning correctly and I refer to
nfoview:
https://salsa.debian.org/python-team/packages/nfoview/-/blob/debian/master/debian/launcher/nfoview

The issue is that I installed git_multimail.py twice in fact it should
be under /usr/lib/python3 only as jcfp said. So i remove it in /usr/bin
by hand.

Now I have removed the launcher for git-multimail and it should work:)

I would also name the binary package `git-multimail` instead of
`python3-git-multimail`, since we don't really care that much that the
thing is written in python, since it's not a library.

Got it. I mixed python module and library here.

(I paste and copy another mail in here:)

Finally, one fundamental issue with this package is that, as you
correctly identified, it's unmaintained upstream. If we do ship it in
Debian, maybe we'd want to keep it out of stable until that is settled?

Ok. I think so also. Fortunately, maintainer of upstream has a little
response, but that's all.

I think that's what I have for now. I haven't double-checked the
upstream branch to see if it matches the upstream repo I have here, but
that would be my next step before uploading... just a formality to make
sure everything matches up.

Thanks for working on this package!

Thanks. This is the first package made by me with you all help.

The new version for git-multimail is here:
https://mentors.debian.net/package/git-multimail/

Thanks again for your encouragement.:)
--
The greatest crimes in the world are not committed by people breaking
the rules but by people following the rules. It's people who follow
orders that drop bombs and massacre villages.
                       - Banksy



--
Regards,
--
  Bo YU

Attachment: signature.asc
Description: PGP signature


Reply to: