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

Re: Bug#1064297: RFS: foolsm/1.0.21-1 -- Link connectivity monitor tool


Em 06/05/2024 11:31, Daniel Gröber escreveu:

Hi Lucas,
Hi d-mentors (there's a workflow question below),

On Sun, Mar 24, 2024 at 05:16:54PM -0300, Lucas Castro wrote:

The source builds the following binary packages:

   foolsm - Link connectivity monitor tool
   lsm - Link connectivity monitor tool - transitional package

To access further information about this package, please visit the following URL:

   https://mentors.debian.net/package/lsm/

Alternatively, you can download the package with 'dget' using this command:

   dget -xhttps://mentors.debian.net/debian/pool/main/l/lsm/lsm_1.0.21-1.dsc

I like using git since it makes dsc review easier. I've converted the
upstream tarball history and your uploads to git using gbp and put them
here:

   https://salsa.debian.org/debian/lsm


I'm already using gbp, on my own repository server
https://git.gnuabordo.com.br/foolsm.git/, I haven't created the salsa account yet. 



If you don't want to use git that's fine it's just a convinience for the me
or the next DD to sponsor lsm but I'd be happy to help you figure out the
Debian git workflow if you want.

Package Review
--------------

d/changelog:

lsm (1.0.21-1) unstable; urgency=medium
.
   * New upstream release (Closes: #1041221)
   * Usrmerge compliance (Closes: #1054086)

Could be more specific. "Use dh_installsystemd to install units" maybe?


   * Package rename

Use imperative in changelogs and be more specific: "Rename package to
'foolsm' to stay consistent with upstream" or some such.


      - Added transitional package for renaming process

More specific please. I'd go with straight prose and not bullet-points
myself. Say: "The old 'lsm' package is now transitional and installs the
new 'foolsm' package.


      - Debian package was modified after upstream project rename.

I'm not sure what you're trying to tell me here?


   * debian/watch updated
   * debian/patches/ cleaned out

IMO these are implied. Ofc. we're going to do that when we update a package
in Debian so while these would make good git commits I don't think they
should be in d/changelog since that's mostly user-facing.

Maybe that's just my git sensibilities showing and it's perfectly
appropriate to note this in d/changelog for the old dsc focused workflow?
Feel free to rebuttle this point.


d/copyright:

Files: *
Copyright: 2009-2016 Mika Ilmaranta <ilmis@nullnet.fi>
	     2009-2015 Tuomo Soini <tis@foobar.fi>

licensecheck finds newer copyright dates, some ftp reviewers like to be
pedantic here and since we'll make a trip through NEW its best to be exact
here, should be:

     Copyright: 2009-2019 Mika Ilmaranta <ilmis@nullnet.fi>
                2009-2021 Tuomo Soini <tis@foobar.fi>

Yep, Forgot about this copyright update.



d/rules:

DEBUG=true

I'm not sure how to feel about this. Do you have a reason for turning it
on? Seems the last upload had it commented out. From a quick codereivew it
does look to just add more logging, so it's probably fine?

Ops, built upon wrong git branch. = ) I'm going upload a new package.



Looking at the generated maintainerscripts in the foolsm deb I don't see
anything related to dpkg-maintscript-helper, are you sure that's hooked up
right? Good job finding that obscurica BTW I didn't know about that myself
:)

that's applied for transitional package, conf path update.

man says:


When using a packaging helper, please check if it has native
dpkg-maintscript-helper integration, which might make your life
easier. See for example dh_installdeb(1).

Hmm. I do see:

     $ cat debian/lsm.preinst.debhelper
     # Automatically added by dh_installdeb/13.11.4
     dpkg-maintscript-helper mv_conffile /etc/lsm/lsm.config /etc/foolsm/foolsm.conf 1.0.21\~ -- "$@"
     # End automatically added section

but that somehow doesn't seem to make it into the deb. Oh. The
lsm.maintscript probably has to be called s/lsm/foolsm/ for it to work.
Nope, the maintscript is right, should be ran just for lsm update, or somehow the lsm is installed but foolsm is available.
The script will check if /etc/lsm/lsm.conf already exist, then it'll move the conf file. 




Random notes:

I also noticed the upstream tarballs have started including a debian/
directory for a non-native packaging. Do you know what's up with that? I
could see why they would include it if they packaged it as a "native"
package but for non-native it just makes no sense. Could you talk to
upstream to figure out what's up with that? Feel free to CC me.

My guess is they would try update the package because I had missed.



Just FYI: I'd appreciate git commits/patches on top of my repo above
instead of an updated dsc dump.
As I mentioned, I don't have a salsa account, I really would like to keep my own repository by now, maybe soon I'll create an account. 




Thanks,
--Daniel


Thanks.

Attachment: OpenPGP_0x42F79A5E0A4D5598.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature

Reply to: