Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Post 0.107 cleanup & dropping API/ABI compat #400

Merged
merged 16 commits into from
Jan 16, 2024
Merged

Conversation

slyon
Copy link
Collaborator

@slyon slyon commented Aug 21, 2023

Description

Note

Please review commit-by-commit (and ignore the huge "ABI: regenerate compat check for dropped symbols" commit in abi-compat/*.xml -- it's autogenerated)

Unrelated changes

This PR contains some unrelated "cleanup" changes for post 0.107 tasks.

Primary changes

The primary changes, of dropping legacy Netplan API and ABI (in preparation for a breaking 1.0 release) start from commit "test:generator:base: Refactor to use new API".

Checklist

  • Runs make check successfully.
  • Retains 100% code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad.

@daniloegea
Copy link
Collaborator

Few more things to remember:

@slyon
Copy link
Collaborator Author

slyon commented Aug 21, 2023

see #401

@slyon slyon force-pushed the post-0107 branch 8 times, most recently from 3796fb9 to e6e8d20 Compare August 30, 2023 07:47
@slyon slyon changed the title Post 0.107 cleanup Post 0.107 cleanup, prepare 1.0 Jan 10, 2024
@slyon slyon force-pushed the post-0107 branch 2 times, most recently from 5c3fda5 to 706ccf9 Compare January 11, 2024 14:34
@slyon slyon changed the title Post 0.107 cleanup, prepare 1.0 Post 0.107 cleanup & dropping API/ABI compat Jan 11, 2024
@slyon slyon force-pushed the post-0107 branch 4 times, most recently from b11fec3 to dd71696 Compare January 11, 2024 15:13
@slyon slyon marked this pull request as ready for review January 11, 2024 15:29
@slyon slyon requested a review from daniloegea January 11, 2024 15:29
Copy link
Collaborator

@daniloegea daniloegea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's looking good so far. The only thing we must fix are the tests that are failing. They just need to be zeroed because the \0 is missing when the strings are compared (see my comment in the test).

tests/generator/base.py Show resolved Hide resolved
tests/ctests/test_netplan_misc.c Outdated Show resolved Hide resolved
src/util-internal.h Outdated Show resolved Hide resolved
@slyon slyon force-pushed the post-0107 branch 8 times, most recently from 7dd4f86 to c152ea4 Compare January 16, 2024 13:29
@slyon
Copy link
Collaborator Author

slyon commented Jan 16, 2024

Thanks for your review @daniloegea ! I've addressed all of your comments and got the CI to be green (except for the known tunnel/GRE failures on Jammy autopkgtest, which are unrelated).

PTAL.

@slyon slyon requested a review from daniloegea January 16, 2024 15:05
Copy link
Collaborator

@daniloegea daniloegea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@@ -45,7 +45,12 @@ jobs:
pull-lp-source netplan.io
cp -r netplan.io-*/debian .
rm -r debian/patches/ # clear any distro patches
sed -i 's|rm debian/tmp/lib/netplan/generate|# DELETED|' debian/rules
# usrmerge-fix
mkdir debian/extra
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this will start to fail when your changes get synced with Ubuntu but we can fix the CI again later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. And I just uploaded 0.107.1-2 into Debian unstable. I'll use my next PR to adopt CI accordingly, once that landed.

@slyon slyon merged commit 780e8c7 into canonical:main Jan 16, 2024
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants