-
Notifications
You must be signed in to change notification settings - Fork 233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
helper/validation: StringInSlice
: Single quote slice elements
#1237
Conversation
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.
Hi @Frankkkkk 👋 Thank you for submitting this change and makes sense in this case. Please note the associated unit test, TestValidationStringInSlice
, needs to be updated as well:
--- FAIL: TestValidationStringInSlice (0.00s)
strings_test.go:283: expected test case 2 to produce error matching "expected [\w]+ to be one of \[ValidValue AnotherValidValue\], got VALIDVALUE", got [expected test_property to be one of ["ValidValue" "AnotherValidValue"], got VALIDVALUE]
Maintainer note: I think this differs from https://github.com/hashicorp/terraform-plugin-framework-validators/pull/152/files because over there, it was %q
quoting framework types.String
values instead of string
.
Hi @bflad thanks for your feed back. Sorry, I forgot to add the test file to the commit :(. It should be fixed. |
No worries, @Frankkkkk! If you're interested, this change likely should also have a changelog entry which you can add if you are interested. You can find information about adding one in the contributing guide. Something like "helper/validation: Added quoting in |
The validation function's error is ambiguous when dealing with slices containing spaces. For example: ``` expected role_name to be one of [Apache Spark Administrator Synapse Credential User Synapse Administrator] ``` By changing the format verb, the error becomes more user friendly: ``` expected role_name to be one of ["Apache Spark Administrator" "Synapse Credential User" "Synapse Administrator"] ``` This fixes hashicorp#464 Signed-off-by: Frank Villaro-Dixon <[email protected]>
No problem :-) It should be done. |
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.
Looks great, thanks, @Frankkkkk 🚀
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
The validation function's error is ambiguous when dealing with slices containing spaces. For example:
By changing the format verb, the error becomes more user friendly:
This fixes #464