feat: Add more unit tests for default optional template parameter of VirtualConfig<> #39
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
andre-nguyen
changed the title
Schmluk
approved these changes
Jan 13, 2025
There was a problem hiding this comment.
LGTM! Thanks a lot @andre-nguyen for adding these utests, feel free to squash and merge.
Am I supposed to have merge permissions? 😅 |
|
@andre-nguyen Thanks for adding the tests / glad #37 helped with your issue!
Ideally yes (we tend to follow a "whoever submits the PR gets to merge it once approved" approach) but clearly haven't looked too hard at the permissions for the public repos we have. Hopefully we'll figure it out for your next contribution! |
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.
@nathanhhughes Follow up to #37
I think I stumbled on the same problem as you. I wanted an optional
VirtualConfig<Foo>to pass parsing and validation when the config wasn't specified. Its a way for me to disable a certain feature that could have different settings depending on how it should behave.Was tinkering with a unit test only to realize you had already fixed the issue and I was on an older commit. Thought these could still be useful.