CLOUDP-303346: Add support for parameter aliases in OpenAPI spec #3698
Conversation
| @@ -338,6 +344,10 @@ components: | |||
| description: Flag that indicates whether the response body should be in the <a href="https://en.wikipedia.org/wiki/Prettyprint" target="_blank" rel="noopener noreferrer">prettyprint</a> format. | |||
| in: query | |||
| name: pretty | |||
| x-xgen-atlascli: | |||
| flag-short: p | |||
There was a problem hiding this comment.
p is usually a short for password, and we use it in a couple of places to avoid confusion I advise against it
There was a problem hiding this comment.
This is a fixture in a unit test, I don't think the actual values are relevant here.
| @@ -338,6 +344,10 @@ components: | |||
| description: Flag that indicates whether the response body should be in the <a href="https://en.wikipedia.org/wiki/Prettyprint" target="_blank" rel="noopener noreferrer">prettyprint</a> format. | |||
| in: query | |||
| name: pretty | |||
There was a problem hiding this comment.
does pretty have any meaning for the CLI?
There was a problem hiding this comment.
This is a fixture in a unit test, I don't think the actual values are relevant here.
There was a problem hiding this comment.
as in isn't the CLI always pretty?
There was a problem hiding this comment.
oh interesting seems like no after trying thi s
There was a problem hiding this comment.
But to reply to your question, it does not have a meaning to the CLI; it's just a regular parameter passed to the backend, and the CLI doesn't handle it any differently.
tools/api-generator/convert.go
Outdated
| short: "", | ||
| } | ||
|
|
||
| getExtensionValues := func(extensionsMap map[string]any) { |
There was a problem hiding this comment.
any particular reason for an anonymous function? doesn't seem to be used as a callback or similar pattern of passing as an arg
There was a problem hiding this comment.
Because of the captured ext variable and trying to avoid filling the namespace with all similarly named functions.
There was a problem hiding this comment.
that's probably a tell we may need better code organization, in lining the function because trying to name it causes confusing with other functions sound like a good time to think if this package needs some cleaning
There was a problem hiding this comment.
I've reorganized the code to get rid of the anonymous function
tools/api-generator/commands.go.tmpl
Outdated
| @@ -43,6 +43,7 @@ var Commands = GroupedAndSortedCommands{ | |||
| IsArray: {{ .Type.IsArray }}, | |||
| Type: `{{ .Type.Type }}`, | |||
| }, | |||
| Aliases: []string{ {{ range $i, $alias := .Aliases }}{{if $i}},{{end}}`{{ $alias }}`{{end}} }, | |||
There was a problem hiding this comment.
can this be
| Aliases: []string{ {{ range $i, $alias := .Aliases }}{{if $i}},{{end}}`{{ $alias }}`{{end}} }, | |
| {{if .Aliases }} Aliases: []string{ {{ range $i, $alias := .Aliases }}{{if $i}},{{end}}`{{ $alias }}`{{end}} }, {{end}} |
There was a problem hiding this comment.
I was hesitating between what I have and what you suggested.
I've updated the code.
There was a problem hiding this comment.
I still se a couple of empty slices initialization which I was trying to avoid
There was a problem hiding this comment.
That's fixed now. I blindly accepted the suggestion, but I forgot that there's a second piece of code that does the same thing.
| update: | ||
| x-xgen-atlascli: | ||
| aliases: | ||
| - projectId |
There was a problem hiding this comment.
nit: it works the way it is, but currently, we have a small gap we can't provide the short version of the alias (we only provide --projectId but we don't have a way to provide a hypothetical -P)
There was a problem hiding this comment.
Good suggestion, I'll make the changes!
|
Coverage Report 📈
|
Proposed changes
projectIdalias to allgroupIdparameters.extractExtensionsFromOperationto returnoperationExtensionsinstead of a tuple.Jira ticket: CLOUDP-303346