Hello Tianyu Chen, I've had a quick look at your proposed NMU and have some comments below.
On Tue, Nov 22, 2022 at 12:45:17PM +0800, Tianyu Chen wrote: > Package: sponsorship-requests > Severity: normal > X-Debbugs-Cc: [email protected] > > Dear mentors, > > I am looking for a sponsor for my package "n2n": > > * Package name : n2n > Version : 3.1.1-0.1 > Upstream contact : Luca Deri <[email protected]> > * URL : http://www.ntop.org/products/n2n/ > * License : Expat, __AUTO_PERMISSIVE__, GPL-3.0+ > * Vcs : https://github.com/leggewie-DM/n2n > Section : net > > Though "n2n" is already in debian, but it's already behind upstream(See > #914321 > and #1024139). I'm uploading a NMU for that. [...] > Changes since the last upload: > > n2n (3.1.1-0.1) unstable; urgency=medium > . > * Non-maintainer upload. > * New upstream version 3.1.1. (Closes: #914321, #1024139) First, an NMU is expected to be minimal changes needed. It is thus unexpected to see that you're completely gutting the existing packaging and replacing it with what is essentially a new from-scratch packaging. (specially since debian/changelog gives no clues about this.) I understand that a major new upstream version bump it might not have been possible to preserve much of the old stuff, but still.... it makes it quite hard for the reviewer. I'm also wary about your transition to machine-readable debian/copyright which throws away all existing copyright notices for existing package. I'm not a lawyer, so can't say what's correct.... I guess it can be argued both ways that you've completely replaced debian/ with new stuff or that no matter what it's still a derived work of the existing package. Either way, the new debian/copyright not having any explicit stanza for `Files: debian/*` makes me a bit worried. Also a nitpick, guidance like the below should IMHO be removed once you've done it: ``` # Please double check copyright with the licensecheck(1) command. ``` Many other files also has this comment (which you can also remove once you've followed the advice): ``` # You must remove unused comment lines for the released package. ``` (Another nitpick is that most files in debian/* starts with an empty line.... remove it?!) The old package had a /etc/init.d/n2n and in the new package you've introduced systemd service files.... with different naming! - How will upgrades works for existing users? - you're manually installing from debian/systemd/* (possibly because using template/instance units) which means you get no automatic debhelper integration generating maintscript code for you... and I don't see any manual either. Even if you want to ship disabled-by-default (as from docs there's no default configuration file installed, just a sample), you should likely issue a restart on upgrades atleast. In debian/rules you're manually hooking in dh_update_autotools_config ... when using dh compat 13 this is AFAIK handled automatically. You're also build-deping on cmake while at the same time specifying in debian/rules explicitly that you want to use autotools build system. Why? I'm having a hard time getting through all of this as a reviewer. One trivial example that makes it more annoying then necessary for a reviewer is debian/patches/move-man-pages.patch which makes the debdiff very verbose. Why not just submit something like this to upstream and then pull it in when upgrading to next upstream release?! If you really want to move files around, making a less noisy diff can be accomplished by using mv in debian/rules instead. Then it would be much more descriptive for a reviewer exactly what you're doing (rather than including a patch that is suitable for upstream submission). As is I can't comfortably sponsor this (but maybe someone else having a look has a different view), specially this close to the freeze. My personal recommendation would be that if it's too late to do minor incremental updates then pursue your ITS (rather than a NMU). Also please consider how upgrading existing users will work out. If you see no other way than the local sysadmin doing manual changes, then atleast write about this in debian/NEWS (see man dh_installchangelogs) and possibly also write some guidance in debian/README.Debian about how the integration works in Debian specifically and how the old installations configuration maps to the new. Regards, Andreas Henriksson
