Drop dependency on Net::Interface #217
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Makes installation easier/possible on CentOS and Alpine. Results in a less helpful error message in a rare situation - i.e. when --sourceaddr is specified with an unreasonable value.
matsduf
reviewed
Oct 12, 2021
| documentation => __( 'Local IP address that the test engine should try to send its requests from.' ), | ||
| documentation => __( | ||
| 'Source IP address used to send queries. ' | ||
| . 'Setting an IP address not correctly configured on a local network interface causes cryptic error messages.' |
There was a problem hiding this comment.
I suggest adding "This option is rarely needed."
There was a problem hiding this comment.
I don't see when that would be useful. But I'll add sourceaddr to the POD.
There was a problem hiding this comment.
I don't see when that would be useful.
The proposed text is to further prevent users from using the option. It is really for special cases.
There was a problem hiding this comment.
I expect that the people who use this option are exactly the ones who need it. Even without such an assertion.
There was a problem hiding this comment.
I cannot see the problem of adding extra comment.
matsduf
approved these changes
Oct 27, 2021
ghost
mentioned this pull request
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Results in a less helpful error message in a rare situation - i.e. when
--sourceaddr is specified with an unreasonable value.
Purpose
Makes installation easier/possible on CentOS and Alpine.
Context
This makes it easier to implement #216.
Changes
This PR removes some code to display a more helpful error message in an edge case. To compensate the documentation is improved for the involved CLI parameter.
The dependency on Net::Interface is removed.
How to test this PR
Make sure the usage documentation for the --sourceaddr option clarifies that you need to specify an IP address that is configured on a network interface:
Make sure we don't depend on Net::Interface: