Path: csiph.com!aioe.org!bofh.it!news.nic.it!robomod From: Antoine =?UTF-8?Q?Beaupr=C3=A9?= Newsgroups: linux.debian.bugs.dist,linux.debian.maint.python Subject: Bug#1007025: git-multimail 1.6.0 package review Date: Tue, 20 Sep 2022 17:10:01 +0200 Message-ID: References: X-Original-To: Bo YU X-Mailbox-Line: From debian-bugs-dist-request@lists.debian.org Tue Sep 20 15:00:11 2022 Old-Return-Path: X-Spam-Flag: NO X-Spam-Score: -3.999 Reply-To: Antoine =?UTF-8?Q?Beaupr=C3=A9?= , 1007025@bugs.debian.org Resent-To: debian-bugs-dist@lists.debian.org Resent-Cc: wnpp@debian.org, vimer X-Debian-Pr-Message: followup 1007025 X-Debian-Pr-Package: wnpp Organization: Debian MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Debian-User: anarcat X-Debian-Message: from BTS X-Mailing-List: archive/latest/1735259 List-ID: List-URL: Approved: robomod@news.nic.it Lines: 107 Sender: robomod@news.nic.it X-Original-Cc: 1007025@bugs.debian.org, Bastian Germann , debian-python@lists.debian.org X-Original-Date: Tue, 20 Sep 2022 10:56:05 -0400 X-Original-Message-ID: <87mtau5c3e.fsf@curie.anarc.at> X-Original-References: <878rt78k24.fsf@angela.anarc.at> <8735jf8a0w.fsf@angela.anarc.at> <8735jf8a0w.fsf@angela.anarc.at> <164694434412.23888.15342309351823597252.reportbug@angela.anarc.at> <5062951b-5c19-60a6-daa9-8acde45eaef6@debian.org> <164694434412.23888.15342309351823597252.reportbug@angela.anarc.at> <20220717152756.lyoqizifnfhykqrg@debian> <87illowqce.fsf@emma.anarc.at> <20220918031024.rehgofq4bc4k5hw4@debian> <164694434412.23888.15342309351823597252.reportbug@angela.anarc.at> <20220918031024.rehgofq4bc4k5hw4@debian> Xref: csiph.com linux.debian.bugs.dist:1117989 linux.debian.maint.python:14480 On 2022-09-18 11:10:24, Bo YU wrote: > Hi, > On Thu, Sep 15, 2022 at 05:32:33PM +0100, Antoine Beaupr=C3=A9 wrote: >>Hi! >> >>I've done a quick review of the 1.6.0 package on salsa as of commit >>d5bd184a1cf73b752f80dea46d8080493a5e663b. [...] >>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-12= 45009306 > (To avoid bring noisy for upstream, i just recorded it in a issue) I don't think pull requests are noisy... you should probably just submit this as a PR upstream. [...] >>I'm also surprised we need that launcher at all. Normally, the >>`setup.py` wrapper has a scripts=3D 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/maste= r/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:) Hm. This is weird. Why would you *not* want git-multimail under /usr/bin? I understand the that git_multimail.py *module* should be in /usr/lib/, but you should *also* have a launcher in /usr/bin/ I think, therefore, this is incorrect: +override_dh_python3: + dh_python3 + rm -f debian/git-multimail/usr/bin/git_multimail.py + First off, don't use `-f` there: we *do* want to fail if the file doesn't exist, so that we can remove the override. Second, this looks wrong: `git-multimail` (the launcher) should be the thing that's installed under /usr/bin, not `git_multimail.py` (the module). If the module ends up in /usr/bin, then it's probably a bug upstream that should be filed. Could you clarify what happens with the package right now? [...] >>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. Okay, so the right way to do this is to file a release-critical bug against the package once it enters Debian. >>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.:) I hope that helps! :) a. --=20 Omnis enim ex infirmitate feritas est. All cruelty springs from weakness. - Lucius Annaeus Seneca (58 AD)