-
Notifications
You must be signed in to change notification settings - Fork 21
Update YAML Schema Validation #70
base: develop
Are you sure you want to change the base?
Conversation
return nil | ||
} | ||
|
||
func cloneRepository(url string, path string) 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 believe that this function is unnecessary. The responsibility of cloning the repository should be on the NewPackageHandlerFromURL
method. Subsequent actions should be performed using the methods of the PackageHandler
type returned by it.
|
||
//go:embed schema/* | ||
var fs embed.FS | ||
|
||
func validateYAMLSchema(schemaFile, documentFile string) 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 suggest creating two private methods in PackageHandler
:
checksProfileSchema
checkManifestSchema
return nil | ||
} | ||
|
||
func walkDirectories(root string) 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 believe this function is unnecessary because the Packagehandler
already knows the exact location of the profile.yml
and manifest.yml
files by following the specification, so there is no need to walk through the entire directory tree. Additionally, this function checks for manifest.yml
and profile.yml
files that may not be necessary for the specification, as they might be located in a different directory than the one declared in the spec.
OptionID: | ||
type: object | ||
properties: | ||
name: | ||
type: string | ||
target: | ||
type: string | ||
help: | ||
type: string | ||
type: | ||
const: "id" | ||
default: | ||
type: string | ||
required: | ||
- name | ||
- target | ||
- help | ||
- type | ||
additionalProperties: false |
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.
We deprecated ID, this should be removed
OptionSelect: | ||
type: object | ||
properties: | ||
name: | ||
type: string | ||
target: | ||
type: string | ||
help: | ||
type: string | ||
type: | ||
const: "select" | ||
default: | ||
type: string | ||
validate: | ||
type: object | ||
properties: | ||
options: | ||
type: array | ||
minItems: 1 | ||
items: | ||
type: string | ||
required: | ||
- options | ||
additionalProperties: false | ||
required: | ||
- name | ||
- target | ||
- help | ||
- type | ||
additionalProperties: false |
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.
validate
field must be required for Select
. A Select
without options makes no sense
- $ref: '#/definitions/OptionURI' | ||
- $ref: '#/definitions/OptionSelect' | ||
- $ref: '#/definitions/OptionPort' | ||
- $ref: '#/definitions/OptionID' |
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.
Remove OptionID
, as noted in the above comment.
Update yaml schemas validation
Changes:
Types of changes
Testing
Requires testing, No
In case you checked yes, did you write tests? No
Comments about testing, should you have some (optional)