[cherrybomb] MTecknology's Masterless Contributions #56353
Conversation
ghost
requested a review
from 
|
@MTecknology Thank you for doing this. |
There was a problem hiding this comment.
@MTecknology thanks for the port PR! 🎉
Just one question on it, but otherwise it looks good to me 👍
This patch fixes logic errors made by using `def_addrfam` in assumptions and removes the need to have `dual_stack` logic. It fixes bugs caused by this logic. Issue #46388 is resolved by preventing IPv4-only options from over-writing IPv6 options. This patch also: - adds `-enable_ipv4:` which is meant for future deprecation of IPv4 - makes better use of `-enable_ipv6:` - adds support for `-ipv4<opt>:` prefix (similar to `-ipv6<opt>:`) - adds support for sharing options between IPv4/v6 - supports overriding any IPv4/v6 option - makes `-proto:` a required field BREAKS TESTS - Tests have not been updated for new behavior.
- Unit tests have been updated for fixed/improved behavior: + Bond options are shared between IPv4/v6 + Added now-required `-proto:` + Added test for issue reported in #46388 (loopback v4opt->inet6) - Removed some redundancy - Added tests: + Check for v4/v6-only option sharing/overwriting + Check that netmask/gw is not added unless specified
- Turns list of options into loop, from long list of static if's - Drops the (nearly redundant) separate IPv4/v6 block - Allows all options to be space-delimited list, line-delimited list, or string - Removes extraneous newlines in interfaces file
…ns from networking config.
- Updated debian_ip to read /all/ options for IPv6 interfaces - Updated template to handle a list version of all options - Enabled remaining tests.
|
looks like there are some merge conflicts that need to be resolved and pre-commit is failing. |
|
There's nothing I can do about the pre-commit thing. That looks like it didn't find the revision it was looking for. This has been sitting around untouched for nearly a month. If I fix the merge conflicts, will the changes be merged before it needs to happen again? Should I wait to fix the conflicts? |
|
@MTecknology I just opened a PR against your branch that should address any of the questionable conflicts here. I got my changes by doing this: Then I just cherry-picked that revision on top of your branch. I would keep a branch name pointing to this cherry bomb commit but my PR should solve any code formatting disagreements on this PR 🤞 |
|
I guess Jenkins failed to report this success: https://jenkinsci.saltstack.com/job/pr-macosxmojave-py3/job/PR-56353/25/ |
|
Fixed the merge conflicts (since I was responsible for them Let's see if it works this time! |
)
What does this PR do?
🎵 Hello Salt. Hello Friends. Here's your... ch-ch-ch-cherry bomb! 🎵
This PR cherry-picks of all my previously-accepted PRs which have not been merged from
developtomasterand do not appear to be part of any current merge effort. These previously accepted PRs are also present in the PRs to port to master project board and have been (personally) reviewed for current relevancy/accuracy.The two areas of salt impacted by these changes are: ['Documentation', 'Debian Networking]
What PRs* does this PR fix or reference?
Tests included*?
Yes; I may have gotten a tiny bit carried away with creativity, but I also tried to break my own code and documented even the silly attempts as tests.
Commits signed with GPG?
Yes, absolutely.