Code review comment for ~paelzer/ubuntu/+source/libvirt:merge-8.0-jammy

Revision history for this message
Sergio Durigan Junior (sergiodj) wrote (last edit ):

Thanks for the MP, Christian.

As usual, a big diff to review! I could not find the regular merge tags (reconstruct, split and logical), so it's "a bit" more work to check that the delta is OK. Here's the command I've used (which is not perfect, so I needed to tweak things a bit):

diff -u <(git log pkg/import/7.0.0-1..pkg/import/7.6.0-0ubuntu3 --format=%s -- debian/ | grep -v '^changelog' | grep -v '^update-maintainer' | sort) <(git log pkg/import/8.0.0-1..merge-8.0-jammy --format=%s -- debian/ | grep -v '^changelog' | grep -v '^update-maintainer' | sort)

I'm also happy with the PPA build (I won't build the package locally; takes too long), and I'm happy with the autopkgtest that have been run on the infra (again, I won't run them locally, and TBH running directly on the infra is better anyway).

I double-checked all commits and made sure that they are being properly listed in the d/changelog entry. I'm leaving a few minor comments in the changelog; they're cosmetic, though.

As for the debhelper/systemd change, my only comment is that you're still using the option "--no-stop-on-upgrade" when invoking dh_installsystemd on d/rules. I think it is best to remove that option, because otherwise debhelper will generate the bogus snippet anyway and it may get executed on postinst/prerm. WDYT?

I'm overall satisfied with the MP. If you have the time and can wait, feel free to push the merge tags and I will take a closer look into the delta. Otherwise, I am OK if you'd like to proceed; there's nothing strange that caught my eyes.

review: Approve

« Back to merge proposal