-
Notifications
You must be signed in to change notification settings - Fork 14
CLOUDP-297223: Add support for private and public preview spec generation #432
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
Conversation
| } | ||
|
|
||
| // WithFullContent returns an Option to generate a new APIVersion given the contentType and contentValue. | ||
| func WithFullContent(contentType string, contentValue *openapi3.MediaType) Option { |
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.
core change: we need to consider contentValue to gather version data, since for preview and private preview we have the same content type atlas.vnd.preview+json, we can only differentiate them based on the x-xgen-preview extension
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.
LGTM with one suggestion
| } | ||
|
|
||
| func (v *APIVersion) IsPrivatePreview() bool { | ||
| return strings.Contains(v.version, PrivatePreviewStabilityLevel) |
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.
Why don't you call IsPreviewSabilityLevel(v.version) here?
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.
IsPreviewSabilityLevel returns true to either Public or Private previews so we need to check for private preview here, but open to other suggestions
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.
LGTM. Great work
| } | ||
| } | ||
|
|
||
| func TestSplitPublicPreviewRun(t *testing.T) { |
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 use the command locally to generate the spec and I noticed that the private preview file has several components that I think are not needed. Is this expected?
test-private-preview-info-resource.json
I am wondering if this is okay and will be fixed once we add the filter to remove the unused view 🤔
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.
Yes, we should cover removing unused stuff after split together, for now we're validating we're filtering out the paths, which is the most important resource
| } | ||
|
|
||
| // GetPreviewVersionName returns the preview version name. | ||
| func GetPreviewVersionName(contentTypeValue *openapi3.MediaType) (name string, err error) { |
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.
Does this need to be public? 👀
| func GetPreviewVersionName(contentTypeValue *openapi3.MediaType) (name string, err error) { | |
| func getPreviewVersionName(contentTypeValue *openapi3.MediaType) (name string, err error) { |
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 think yes cause we use it in the versions package
Proposed changes
Jira ticket: CLOUDP-297223
Checklist
Changes to Spectral
Further comments