Repository navigation
refactor/wgcf/optional-keepalive-generation - #637
Open
michaelik1 wants to merge 4 commits into
Open
michaelik1 wants to merge 4 commits into
michaelik1 wants to merge 4 commits into
Conversation
- rewrite TestKeepaliveFlag using maintainer's way of testing with tests struct - resetFlag helper function introduced in PR ViRb3#634;
The test assumed that PersistentKeepalive would always be the last line in the generated profile, which is not guaranteed. The existing test that verifies the full profile content already covers this case.
This file contains hidden or 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
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.
Motivation
The current
--keepaliveflag usesNoOptDefValto default to 25 when no value is provided. However, this introduces inconsistent behavior between the two common ways of passing flag values.For example:
--keepaliveuses the implicit value of 25.--keepalive=25explicitly sets the value to 25.--keepalive 25uses the implicit value of 25, while25is treated as a positional argument rather than the flag's value.This behavior can be confusing, especially since other flags support both the equals and space-separated syntax.
Description
Removes
NoOptDefValfrom the--keepaliveflag, making its value explicitly required whenever the flag is specified.Usage:
The default remains
0when the flag is omitted.The equals syntax remains unchanged, preserving compatibility with the usage documented in the README. However, invoking
--keepalivewithout a value will now produce an error instead of implicitly using 25.Tests
generate_test.goto use table-driven tests covering both flag syntaxes, missing values, and invalid inputs.TestGenerateProfileWithCustomKeepalivetest fromprofile_test.go, as it assumed thatPersistentKeepalivewould always be the last line of the generated profile. This ordering is not guaranteed, and the remaining test already verifies the complete profile content.@ViRb3