Re: [Pkg-javascript-devel] [RFS] node-trust-json-document
Thanks for the feedback! On Fri, Feb 25, 2022 at 2:35 PM Jonas Smedegaard wrote: > > Quoting Ayoyimika Ajibade (2022-02-25 09:16:17) > > I have taken into consideration the information and corrections and > > here is the updated repo > > https://salsa.debian.org/Ayoyimika/node-trust-json-document > > Thanks! > > Looks good - released now, targeted unstable. > > I did make one minor adjustment (too minor to mention in changelog): > > You added a lintian override which was reported as a mismatch on my > system. Not sure why (and your commit message is fine, I don't mean to > say that you should have been more detailed). Maybe you ran an older > lintian than I have installed? Maybe your system has some non-standard > lintian rules added? > > But as said it was really only a minor detail: The other half of your > lintian-overrides commit was correct, and I would have accepted your > change even if you had not messed with lintian overrides at all, since > all those lintian overrides are completely unrelated to your > future-proofing of webpack. > > > - Jonas > > -- > * Jonas Smedegaard - idealist & Internet-arkitekt > * Tlf.: +45 40843136 Website: http://dr.jones.dk/ > > [x] quote me freely [ ] ask before reusing [ ] keep private -- Pkg-javascript-devel mailing list Pkg-javascript-devel@alioth-lists.debian.net https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-javascript-devel
Re: [Pkg-javascript-devel] [RFS] node-trust-json-document
Quoting Ayoyimika Ajibade (2022-02-25 09:16:17) > I have taken into consideration the information and corrections and > here is the updated repo > https://salsa.debian.org/Ayoyimika/node-trust-json-document Thanks! Looks good - released now, targeted unstable. I did make one minor adjustment (too minor to mention in changelog): You added a lintian override which was reported as a mismatch on my system. Not sure why (and your commit message is fine, I don't mean to say that you should have been more detailed). Maybe you ran an older lintian than I have installed? Maybe your system has some non-standard lintian rules added? But as said it was really only a minor detail: The other half of your lintian-overrides commit was correct, and I would have accepted your change even if you had not messed with lintian overrides at all, since all those lintian overrides are completely unrelated to your future-proofing of webpack. - Jonas -- * Jonas Smedegaard - idealist & Internet-arkitekt * Tlf.: +45 40843136 Website: http://dr.jones.dk/ [x] quote me freely [ ] ask before reusing [ ] keep private signature.asc Description: signature -- Pkg-javascript-devel mailing list Pkg-javascript-devel@alioth-lists.debian.net https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-javascript-devel
Re: [Pkg-javascript-devel] [RFS] node-trust-json-document
Hello! I have taken into consideration the information and corrections and here is the updated repo https://salsa.debian.org/Ayoyimika/node-trust-json-document Thanks and Cheers On Sat, Feb 12, 2022 at 2:26 PM Jonas Smedegaard wrote: > > [ adding back mailinglist as recipient ] > > Quoting Ayoyimika Ajibade (2022-02-12 07:00:54) > > > update webpack command > > As part of the migration to webpack5, some options are no more valid > > example is the -d or -p option which was previously referred to set > > the development or production environment respectively, hence the > > change was the functional effect (e.g needed for webpack5), which will > > try to make my commit messages more descriptive. Thanks for putting > > that in. > > Ah, it is news to me that webpack5 does not support the short-form > options. > > Related note: I run a few Debian systems - laptops and servers - and on > all of them I install the package apt-listchanges, which shows package > changes each time I run "apt update && apt update" - which I do at least > once per day (yeah, I should maybe talk to a psyciatrist about that). > By default, apt-listchanges show only entries from NEWS files, but I > reconfigure it with "dpkg-reconfigre apt-listchanges" to also show > Changelog entries. In short, I read (or skim, very quickly) all changes > for all packages on my systems. I learn a lot from informative changelog > messages. > > I find it an amusing little challenge for myself to express changelog > messages to be both short and informative. I am not very good at it - > for inspiration I can recommend looking at changelog messages (and > mailinglist posts) by Simon McVittie - e.g. gtk4: > https://tracker.debian.org/media/packages/g/gtk4/changelog-4.6.0ds1-4 > > I commonly follow one of these patterns for changelog entries: > > * tedious change done > * change done, highlighting additions > * change done, highlighting additions (highlighting omissions) > > Here, I would maybe write (exactly as you - I am no saint, or) like > this: > > * update webpack command, to be forward-compatible with webpack5 > > It is common to fit most possible (i.e. as close to 72 chars) before > wrapping a line. I instead break at natural "pauses" in the text, > called "semantic newlines": https://sembr.org/ > > > > > Change #2: You update DEP-3 header of a patch > > will make necessary changes to the patch > > I suggest to *drop* that patch: It is not necessary, just makes the > patch one line bigger. > > > > > fix install in binary packages > > don't know if that is the right word 'fix' but there were lots of > > errors thrown from dh_missing as files were not properly installed > > even for autopkgtest to use. Please if i may ask why the need for > > another binary file i.e > > libjs-trust-json-document_0.1.4~dfsg-11_all.deb > > I didn't check but am pretty sure that if you did not apply the next > commit to use pkg-js-tools, then dh_missing would be silent. > > So what you "fixed" was your own changes, not something in the previous > package release, and therefore your "fix" does not belong in the > _changelog_ (it is not an activity log). > > Common procedure when building a package is this: > > a) prepare sources > b) configure upstream build system > c) do an upstream "build" > d) do an upstream "install" to debian/tmp > e) copy files from debian/tmp to debian/$PKG > f) finalize packaging > > Node.js packages often provide no upstream "install" task that fits the > Debian needs. > > This package skipped d) and in e) copies directly from source paths. > dh_missing should not complain about that. > > Your later commit introduces pkg-js-tools to handle d) and e), and > dh_missing then (correctly) complains that something weird is going on: > files are copied twice. > > > > > migrate to pkg-js-tools and autopkgtest to handle test suites > > sure i will revert the change, as i wanted to test with autopkgtest. > > from my limited knowledge, I switched to pkg-js packaging style. Is > > there any way I can do that? > > Make good sense to improve test coverage. > > Yes, it is possible to use autopkgtest without pkg-js. Not as easy, but > possible more reliable. > > autopkgtest already runs a few superficial tests now, before your > packaging changes: It is setup in the directory debian/tests/ > > I guess what you want is to _improve_ autopkgtest, to not only do > superficial tests but also check upstream testsuite (which is currently > checked only during build). > > Upstream testsuite is not designed to check Debian-installed code. Maybe > pkg-js magically ensures this, but possibly it accidentally checks > source code instead, which is less useful. > > One example of a package that checks upstream Node.js testsuite and > ensures that system-installed module gets loaded (not local code) is > node-lunr: run "apt source node-lunr" and see debian/tests/control > > Beware that the upstream testsuite for this package seemingly does *not* > test
Re: [Pkg-javascript-devel] [RFS] node-trust-json-document
[ adding back mailinglist as recipient ] Quoting Ayoyimika Ajibade (2022-02-12 07:00:54) > > update webpack command > As part of the migration to webpack5, some options are no more valid > example is the -d or -p option which was previously referred to set > the development or production environment respectively, hence the > change was the functional effect (e.g needed for webpack5), which will > try to make my commit messages more descriptive. Thanks for putting > that in. Ah, it is news to me that webpack5 does not support the short-form options. Related note: I run a few Debian systems - laptops and servers - and on all of them I install the package apt-listchanges, which shows package changes each time I run "apt update && apt update" - which I do at least once per day (yeah, I should maybe talk to a psyciatrist about that). By default, apt-listchanges show only entries from NEWS files, but I reconfigure it with "dpkg-reconfigre apt-listchanges" to also show Changelog entries. In short, I read (or skim, very quickly) all changes for all packages on my systems. I learn a lot from informative changelog messages. I find it an amusing little challenge for myself to express changelog messages to be both short and informative. I am not very good at it - for inspiration I can recommend looking at changelog messages (and mailinglist posts) by Simon McVittie - e.g. gtk4: https://tracker.debian.org/media/packages/g/gtk4/changelog-4.6.0ds1-4 I commonly follow one of these patterns for changelog entries: * tedious change done * change done, highlighting additions * change done, highlighting additions (highlighting omissions) Here, I would maybe write (exactly as you - I am no saint, or) like this: * update webpack command, to be forward-compatible with webpack5 It is common to fit most possible (i.e. as close to 72 chars) before wrapping a line. I instead break at natural "pauses" in the text, called "semantic newlines": https://sembr.org/ > > Change #2: You update DEP-3 header of a patch > will make necessary changes to the patch I suggest to *drop* that patch: It is not necessary, just makes the patch one line bigger. > > fix install in binary packages > don't know if that is the right word 'fix' but there were lots of > errors thrown from dh_missing as files were not properly installed > even for autopkgtest to use. Please if i may ask why the need for > another binary file i.e > libjs-trust-json-document_0.1.4~dfsg-11_all.deb I didn't check but am pretty sure that if you did not apply the next commit to use pkg-js-tools, then dh_missing would be silent. So what you "fixed" was your own changes, not something in the previous package release, and therefore your "fix" does not belong in the _changelog_ (it is not an activity log). Common procedure when building a package is this: a) prepare sources b) configure upstream build system c) do an upstream "build" d) do an upstream "install" to debian/tmp e) copy files from debian/tmp to debian/$PKG f) finalize packaging Node.js packages often provide no upstream "install" task that fits the Debian needs. This package skipped d) and in e) copies directly from source paths. dh_missing should not complain about that. Your later commit introduces pkg-js-tools to handle d) and e), and dh_missing then (correctly) complains that something weird is going on: files are copied twice. > > migrate to pkg-js-tools and autopkgtest to handle test suites > sure i will revert the change, as i wanted to test with autopkgtest. > from my limited knowledge, I switched to pkg-js packaging style. Is > there any way I can do that? Make good sense to improve test coverage. Yes, it is possible to use autopkgtest without pkg-js. Not as easy, but possible more reliable. autopkgtest already runs a few superficial tests now, before your packaging changes: It is setup in the directory debian/tests/ I guess what you want is to _improve_ autopkgtest, to not only do superficial tests but also check upstream testsuite (which is currently checked only during build). Upstream testsuite is not designed to check Debian-installed code. Maybe pkg-js magically ensures this, but possibly it accidentally checks source code instead, which is less useful. One example of a package that checks upstream Node.js testsuite and ensures that system-installed module gets loaded (not local code) is node-lunr: run "apt source node-lunr" and see debian/tests/control Beware that the upstream testsuite for this package seemingly does *not* test webpack-generated code, only uncompressed code - so even though including upstream testsuite with autopkgtest _is_ a good improvement to the packaging, it will _not_ help tracking if webpack migration works or not. The purpose of webpack is generally to make JavaScript usable in a browser, so testing that in principle requires loading the code in a browser as well. test-friendly
Re: [Pkg-javascript-devel] [RFS] node-trust-json-document
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 aaa 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
[Pkg-javascript-devel] [RFS] node-trust-json-document
Hi There! 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 OpenPGP_0x1FF1115A4CAC464D.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature -- Pkg-javascript-devel mailing list Pkg-javascript-devel@alioth-lists.debian.net https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-javascript-devel
