Code review comment for ~nicolasbock/netplan:tristate-jammy

Revision history for this message
Lukas Märdian (slyon) wrote :

Thank you very much Nicolas for preparing a debdiff/merge-proposal on top of the currently pending netplan Jammy SRU changeset!

The suggested patch is looking good so far, but I left a few inline comments (see below). Furthermore, there might be some misunderstanding of how to apply distro patches. We do not want to modify any files of the "upstream source" in this repository (e.g. src/ doc/ netplan/ ...), but ONLY files in the debian/ packaging subdirectory. Please revert those changes to the upstream source, the new "distro-patch" (quilt) you added in debian/patches/0003-Add-tristate-type-for-offload-options-LP-1956264-270.patch will apply those changes for us during the package build.

Another thing: You missed to add the "ethtool" test-dependency to the "ethernets" test in debian/tests/control, this needs to be added in order to make the new offloading autopkgtests PASS (as has been done here: http://launchpadlibrarian.net/601540356/netplan.io_0.104-0ubuntu2_0.104-0ubuntu3.diff.gz).

Some of my suggested changes/fixes:
https://paste.ubuntu.com/p/vKJR7yMN92/

Last but not least, please adopt the bug report in LP: #1956264 according to the SRU template in https://wiki.ubuntu.com/StableReleaseUpdates#SRU_Bug_Template, describing a proper test case/reporducer and impact analysis, to make it fit for SRU.

Keep up the good work!

review: Needs Fixing

« Back to merge proposal