-
Notifications
You must be signed in to change notification settings - Fork 75
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
Disallow dotted fields #591
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.
Looks good, added a couple of nitpicks.
@@ -72,24 +73,26 @@ func (s *FileSchema) Validate(fsys fs.FS, filePath string) ve.ValidationErrors { | |||
return nil // item content is valid according to the loaded schema | |||
} | |||
|
|||
func loadItemSchema(fsys fs.FS, path string, contentType *spectypes.ContentType) ([]byte, error) { | |||
func loadItemSchema(fsys fs.FS, path string, contentType *spectypes.ContentType, specVersion *semver.Version) ([]byte, 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.
No need to pass by pointer here, right?
func loadItemSchema(fsys fs.FS, path string, contentType *spectypes.ContentType, specVersion *semver.Version) ([]byte, error) { | |
func loadItemSchema(fsys fs.FS, path string, contentType *spectypes.ContentType, specVersion semver.Version) ([]byte, 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.
True, it can be passed by value. I'll change it
} | ||
if contentType != nil && contentType.MediaType == "application/x-yaml" { | ||
return convertYAMLToJSON(data) | ||
return convertYAMLToJSON(data, specVersion.LessThan(semver.MustParse("3.0.0"))) |
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. Consider extracting the parsed version to a constant.
💚 Build Succeeded
History
cc @mrodm |
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.
⛵
Awesome! 🏆 |
What does this PR do?
This PR disallows to use fields with dots. For instance:
Those files should be updated to:
Why is it important?
Allowing dotted fields leads to ambiguous specifications.
Checklist
test/packages
that prove my change is effective.spec/changelog.yml
.Related issues