tools/importer-rest-api-specs: support for pulling the Display Name for an Integer Constant if defined
#3611
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
… for an Integer Constant if defined This commit fixes #3010 by checking for a Display Name within the `x-ms-enum` block and pulling from that if a (Display) Name is defined for an Integer constant. Whilst I considered doing this for Float and String constants too, I've intentionally omitted those since Float constants are rare (so we can apply this same fix if needed down the line) - and for String constants there's little benefit to changing at this point, since we normalize a key from the value for the constant. Ultimately this should ensure that when we import the constant `EmailChannelAuthMethod` for `BotService` that we output this as: > `Password`: `0` > `Graph`: `1` Which is far more descriptive than: > `0`: `0` > `1`: `1`
tombuildsstuff
added
tool/importer-rest-api-specs
Breaking ChangesNo Breaking Changes were found 👍 |
Summary of ChangesNo Breaking or Non-Breaking Changes were found 👍 |
New Resource ID Segments containing Static IdentifiersNo new Resource ID Segments containing Static Identifiers were identified in the set of changes 🤙. |
mbfrahry
approved these changes
Jan 12, 2024
| items := v.([]interface{}) | ||
| displayNameOverrides := make(map[interface{}]string) | ||
| for _, itemRaw := range items { | ||
| item := itemRaw.(map[string]interface{}) |
There was a problem hiding this comment.
Do we want to check that this isn't nil or is it fine?
There was a problem hiding this comment.
The extension is defined in this manner, and everything we're importing matches it - so I think this is fine as-is for now. If this is an issue we can fix it at that point, but that's likely going to be bad data - so would want resolving upstream anyway - as such this is fine 👍
tombuildsstuff
deleted the
f/support-for-using-display-names-for-constants
branch
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.
This commit fixes #3010 by checking for a Display Name within the
x-ms-enumblock and pulling from that if a (Display) Name is defined for an Integer constant.Whilst I considered doing this for Float and String constants too, I've intentionally omitted those since Float constants are rare (so we can apply this same fix if needed down the line) - and for String constants there's little benefit to changing at this point, since we normalize a key from the value for the constant.
Ultimately this should ensure that when we import the constant
EmailChannelAuthMethodforBotServicethat we output this as:Which is far more descriptive than:
There's a couple of changes to the existing data, but it's nothing major: