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

Add basic routes to migration #41

Merged
merged 1 commit into from
Nov 27, 2023
Merged

Conversation

jcronenberg
Copy link
Owner

@jcronenberg jcronenberg commented Nov 23, 2023

Problem

Only basic gateway migration

Solution

Add routes migration
Development note: I got rid of the gateway parsing because essentially adding a route has the same effect. If there is a single gateway it would probably be nicer to add it as a gateway but I wanted to keep it simple (stupid 🙂). Let me know if you disagree.

Fixes #13

Testing

  • Existing unit test
  • Added new integration test
  • Tested manually

@jcronenberg jcronenberg added enhancement New feature or request migration Something needs to be changed/implemented in the wicked migration labels Nov 23, 2023
@jcronenberg jcronenberg requested a review from cfconrad November 23, 2023 17:50
Copy link

github-actions bot commented Nov 23, 2023

Pull Request Test Coverage Report for Build 7004642045

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 7 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+2.6%) to 50.709%

Files with Coverage Reduction New Missed Lines %
rust/src/interface.rs 7 86.34%
Totals Coverage Status
Change from base Build 6972644851: 2.6%
Covered Lines: 143
Relevant Lines: 282

💛 - Coveralls

Copy link
Collaborator

@cfconrad cfconrad left a comment

Choose a reason for hiding this comment

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

I agree with your simplicity!
I only would use 0.0.0.0/0 instead of <gatewayip>/0 for the default gateways destination. I think it's more common.

Copy link
Collaborator

@cfconrad cfconrad left a comment

Choose a reason for hiding this comment

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

LGTM

@cfconrad cfconrad merged commit c897466 into wicked-nm-migration Nov 27, 2023
4 checks passed
@jcronenberg jcronenberg deleted the basic-routes branch November 27, 2023 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request migration Something needs to be changed/implemented in the wicked migration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support adding basic routes for migration
2 participants