This repository has been archived by the owner on Dec 8, 2022. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 147
feat: Implement oneof tag #103
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
5e07ae6
move custom tag check to inside userDefinedArray (following similar p…
andrew-werdna 8b03abb
go fmt fixes; add support for 'oneof' tag for strings; add one basic …
andrew-werdna 86c668c
add error check and error return for malformed tag; add extra tests t…
andrew-werdna 61e4b06
added support for a tag called oneof for strings and ints
andrew-werdna 48bc5e7
update the creation of the custom types (not using the underlying types)
andrew-werdna 6601d1c
update examples; add const for convenience
andrew-werdna e6329a2
go fmt
andrew-werdna 0267e71
update documentation examples; add TrimSpace to ensure space between …
andrew-werdna 2c3d65f
update readme with caveat about 'oneof' tag
andrew-werdna c35890a
linter fixes
andrew-werdna 1a9fe1f
clean up duplicate if conditions
andrew-werdna 668e63e
Merge remote-tracking branch 'origin/master' into setup-OneOf-tag
andrew-werdna cca5c35
finish fixing merge conflicts
andrew-werdna cfb8004
update example to be correct. use ':' to separate arguments not ','
andrew-werdna 210f21c
update example comments for clarity
andrew-werdna 0c1fccd
ensure comma separated arguments to oneof tag; add several more tests…
andrew-werdna fbde531
update documentation examples
andrew-werdna 4c44fc2
remove useless if statement that is never true
andrew-werdna File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any reasons why we choose colon for separator?
From the issue, he expects some kind of
foo,bar,x,y
I'm okay with either, but it will be great if we also considering the user experience?How usually people separate a list of items? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I thought about that. I can switch it if you want. I basically was lazy and didn't want to do the work to make
oneof: value1, value2, value3
. I didn't have an actually good reason for that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to run an errand for a little bit right now. But when I get back in an hour or 2, I will be happy to make the user experience better.
i.e. support instead
oneof: value1, value2, value
<- use colon for theoneof
tag only. Then use commas to actually separate the "arguments" to the tag.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bxcodec I think I have it in place now. It now uses
,
to separate the "arguments" to theoneof
tagThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks bro @andrew-werdna