[Pkg-javascript-devel] [RFS] node-trust-json-document

Sat Feb 12 00:00:06 GMT 2022

Hi Ayoyimika,

Quoting Ayoyimika Ajibade (2022-02-11 19:50:01)
> I have updated node-trust-json-document in version 0.1.4. The package 
> is available at 
> https://salsa.debian.org/Ayoyimika/node-trust-json-document and i am 
> requesting for sponsorship and i ensured all tests passed with 
> autopkgtest, it's lintian clean and built it in a clean chroot with 
> sbuild.

Thanks for looking into the node-trust-json-document.

I agree that the package "clean" but some changes I don't understand the 
purpose of, and some changes I outright disagree with.

Let me try go through the changes one git commit at a time:

> update webpack command

You change the webpack commands to use GNU-style double-dash options.

I like that: I generally strive towards my code being readable by others 
than myself, and (when I remember to do it) I choose GNU-style options 
for that reason.

But I am curious: Is that your reason for this change as well, or is it 
perhaps required with newer webpack (which I know you are working on)?

I don't mean to imply that you should have been more verbose with your 
git commit message.  Sure, ideally a commit message contains a short 
one-line description *and* a short essay e.g. elaborating on reasons for 
the change, but I am sloppy mostly write only a(n often horribly long) 
one-liner myself.  So I really mean that I am just curious if there is 
something for me to learn in this commit or it is a purely stylistic 
change.

> update lintian-overrides

This I disagree with: You do not "update" but totally *rewrite* - 
without explaining why you throw away the existing lintian overrides, 
which at least the copyright-related ones I am strongly confident are 
perfectly valid.

> update copyright & patch

Here you do two independent things in same git commit - better if you do 
do "atomic commits", i.e. commit one semantic change at a time: Easier 
to discuss, and easier to work with (e.g. cherry-pick or revert) later.

#1: You replace a License-Reference field with a (common but still) 
custom-written boilerplate in a debian/copyright file License section. 
Many has done the *exact* same type of change as you did here - it is 
quite common to write debian/copyright files that way, it is so common 
that lintian even complains if not doing it that way - but I happen to 
disagree with that, and that's the reason for one of the lintian 
overrides you threw away in your previous commit.

Debian Policy requires debian/copyright file to contain upstream 
copyright information verbatim.  What you did (and most Debian 
developers does aaallll the time) is not *verbatim* but *cargo cult*: 
Upstream did not write that text - _you_ made that up yourself (or 
copied something that other package maintainers made up)!

In my opinion, text in debian/copyright file made up by the package 
maintainer belongs in Comment: fields.  This is the reason I invented 
the custom License-Reference: field (which in recent time I have changed 
to the shorter Reference:) - to distinguish upstream verbatim content 
from references to upstream facts - while restricting maintainer content 
to Comment: fields.

Sorry, this is confusing and complex also for most Debian developers 
(myself included) - simplest would be if you simply hadn't touched it, 
but I do imagine that when lintian is yelling about it then it is easily 
assumed that lintian gotta be right.

Lintian is not a truth teller, it is only a helper tool!

Change #2: You update DEP-3 header of a patch.  The added line is 
technically correct in itself (yes, I did not forward that patch), 
however a) stating that a patch is not forwarded is implied by the patch 
not having a Bug: field, and b) when DEP-3 headers are changed then also 
the Last-Updated: entry should be updated as well.  These two rules are 
documented in the DEP-3 web page referenced just below your change.

Long story short: Correct but needless change applied slightly 
incorrect.  Either drop the change of also update Last-Updated: entry.

> fix install in binary packages

This is not a fix, there was nothing broken before this change...

> migrate to pkg-js-tools and autopkgtest to handle test suites

...but this change breaks things to require the "fix" previously.

This is an invasive change.  Possibly it is perfectly valid, but shifts 
packaging style from using debhelper to using pkg-js magic.

I am not confortable using the pkg-js packaging style, so this change 
should either be reverted or my name removed from the Uploaders: field - 
in which case you should consult the replacement Uploaders about 
releasing the package. :-/

Sorry, I will not release the package you prepared, despite it 
technically being valid: I don't find the changes needed, and some of 
them even (subjectively) a regression :-(

I sincerely hope you don't take this the wrong way: I can see how all 
the changes you made could be driven by "hmm, this looks odd, let's 
apply common patterns to it".  Just happens that quite a few "oddities" 
were deliberate - except perhaps the short-form webpack options, which I 
am genuinely curious if that has a functional effect (e.g neede for 
webpack5) or was just a stylistic change.

 - Jonas

-- 
 * Jonas Smedegaard - idealist & Internet-arkitekt
 * Tlf.: +45 40843136  Website: http://dr.jones.dk/

 [x] quote me freely  [ ] ask before reusing  [ ] keep private
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: signature
URL: <http://alioth-lists.debian.net/pipermail/pkg-javascript-devel/attachments/20220212/6a486ee3/attachment.sig>