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.
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
MIWI API endpoints #3608
MIWI API endpoints #3608
Changes from all commits
46db8d6
82958cc
ae00e62
288c1c7
3bf910c
3d74980
4e214cf
cb91f12
3348774
61d643c
3cb6965
b3cccb1
dd592e1
8c76e96
f1f1569
3bf8c1f
071294b
93f970f
15a1fde
8476463
dbf6867
7f67b55
90d2819
36e847c
35274fb
d7e58e2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Nit: space between
PlatformWorkloadIdentityRole
and{
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'd put a space before the curly brace if for example this were line 23 and I had left out the space after the
range
expression, but I think it's normal Go style to avoid leaving blank space in that spot in a struct literal, and that's what's done throughout the codebase.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.
Instead of comparing struct fields individually, consider using this: https://github.com/go-playground/validator
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 played around with it, but I don't think it's good for our static validators simply because it doesn't give the user friendly error messages that we want going to those making API requests. For example:
This error message would be perfectly fine for me as a regular developer of the RP, but it doesn't even include the fully qualified path of the API field presenting the issue, which won't be great for external users. (Well, I say "external users", but this is the admin API... 🙂)
I think adding it in just for this PR would create an inconsistency with respect to our error messaging and that with that in mind it would be best considered as a future refactor that we do across all of the static validators. Your thoughts?