Remove enum validation for embedded app name #102
Closed
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
Without this, not specifying embeeded app name errors because it does not adhere to the enum validation. Signed-off-by: Gregory Haynes <[email protected]>
greghaynes
force-pushed
the
fix-appname-optional
branch
from
19b48e4 to
eba8ba5
Compare
making optional doesnt have any effect - unset values result in validation failure. Signed-off-by: Gregory Haynes <[email protected]>
greghaynes
changed the title
Signed-off-by: Gregory Haynes <[email protected]>
greghaynes
force-pushed
the
fix-appname-optional
branch
from
5d676bb to
d6d36a3
Compare
nabuskey
reviewed
Dec 6, 2023
There was a problem hiding this comment.
We can remove enum but I thought it's a nice way to tell what apps are embedded instead of digging through the controller logic here:
I think I should have specified the omitempty json tag when I implemented this. This was corrected in the custom package PR here: https://github.com/cnoe-io/idpbuilder/pull/100/files. It does work from my testing.
|
oh interesting, I'll try that out - I thought omitempty was only for reading though (dont display an empty field)? I'm totally fine with enum as long as we can make it work, as is I am unable to set EmbeddedAppName to "" though, so as long as we enable this I'm happy. |
|
Closing this since it should be resolved in the PR: #100 |
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.
Without this, not specifying embeeded app name errors because it does not adhere to the enum validation. I validated that changing the order does not work as well.