search

voxpupuli / puppet-network

Types and providers to manage network interfaces
https://forge.puppet.com/puppet/network
Apache License 2.0
66 stars 107 forks source link

change name of debian package from ifenslave-2.6 to ifenslave #306

Closed hbog closed 1 year ago

hbog commented 1 year ago

Starting from Debian 9, ifenslave-2.6 is marked as 'transitional package, use ifenslave'. In debian 11 and 12, the ifenslave-2.6 package is no longer available, causing puppet run failure. https://packages.debian.org/search?searchon=names&keywords=ifenslave

Pull Request (PR) description

Update the name of the ifenslave package.

This Pull Request (PR) fixes the following issues

Fixes #305 -->

smortex commented 1 year ago

LGTM!

The ChangeLog being generated from GitHub merged pull-requests, can you please:

  1. remove the 2nd commit from this PR (this PR would then be a bugfix) git push -f nbog-fork bb76f7214b6b9b4ff49047eed99db2f90986353d:ifenslave;
  2. create a new PR that just drop Debian 8 (this PR would be backwards-incompatible);
  3. create a new PR that add support for Debian 10 & 11 (this would be en enhancement PR);

Debian 9 being EOL, no need to add it :-)

Thank you!

hbog commented 1 year ago

Thanks for accepting this PR. The 2nd commit was added to the PR because changing the package name (1st commit) effectively breaks Debian 8 support and adds Debian >=11 support. Hence also the first commit is 'backwards-incompatible'. It is also more an enhancement then a bugfix as the issue it solves exists only in Debian >=11. Note that the 2nd commit only affects the metadata.json file.

Wouldn't it be better to squash both commits ?

smortex commented 1 year ago

Ah, so ordering matter. I think this can consist of 2 PR:

These 2 commits can be one on top of the other, each with it's own branch pointing on it, and a corresponding PR.

hbog commented 1 year ago

Can do, but I think there is still a misunderstanding. The name change is not a detail. bb76f72 adds Debian 11/12 support (by changing the name of a package) and removes Debian 8 support (because the new package name is not known in Debian <=8). Enhancement and backwards-incompatibility cannot be separated in two commits.

08e5c70 documents this in the metadata.json file.

hbog commented 1 year ago

Created a new pull request #308 which replaces this one.