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

Remove duplicate branches in request of plugins #496

Merged

Conversation

hrzlgnm
Copy link
Contributor

@hrzlgnm hrzlgnm commented Sep 23, 2024

The check for whether the name contains "ipv6" is not necessary for the plugins "dnshome" and "ipv64", as the code path for both branches is the same.
Furthermore the check is rather confusing in the context of the provider named "ipv64.net", as "ipv6" is also contained in the provider name itself.

The check for whether the name contains "ipv6" is not necessary for
the plugins "dnshome" and "ipv64", as the code path for both branches is the
same.
Furthermore the check is rather confusing in the context of the
provider named "ipv64.net", as "ipv6" is also contained in the
provider name itself.
@hrzlgnm
Copy link
Contributor Author

hrzlgnm commented Sep 24, 2024

I guess the same confusion applies to other constructs like for example in

DO(http_init(client, "Checking for IP# change", strstr(provider->system->name, "ipv6") ? TCP_FORCE_IPV6 : TCP_FORCE_IPV4));
, causing ipv4 only updates fetching an ipv6 address from the default check server of the plugin for the provider ipv64.net. I would suggest to refine the check to check the prefix only, not the whole name.
Shall I open a separate issue for that case?

@troglobit
Copy link
Owner

Shall I open a separate issue for that case?

Yes, please do. Need to tread carefully around that legacy behavior, so that needs more review.

@troglobit troglobit merged commit da1d1af into troglobit:master Sep 24, 2024
4 checks passed
@hrzlgnm hrzlgnm deleted the feat/cleanup-duplicate-branches-in-plugins branch September 24, 2024 11:49
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