Skip to content

bugfix: ignore extra fields in yaml#107

Merged
niemeyer merged 2 commits intocanonical:mainfrom
letFunny:ignore-yaml-extra-fields
Dec 13, 2023
Merged

bugfix: ignore extra fields in yaml#107
niemeyer merged 2 commits intocanonical:mainfrom
letFunny:ignore-yaml-extra-fields

Conversation

@letFunny
Copy link
Collaborator

  • Have you signed the CLA?

Ignoring extra fields makes the code forward compatible with changes in the yaml format, without a version bump being necessary.

Ignoring extra fields makes the code forward compatible with changes in
the yaml format, without a version bump being necessary.
Copy link

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Good fix.

type yamlRelease struct {
Format string `yaml:"format"`
Archives map[string]yamlArchive `yaml:"archives`
Archives map[string]yamlArchive `yaml:"archives"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed.

Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. This also propagates to PathInfo right? (e.g. /path: {badkind: foo})

@cjdcordeiro cjdcordeiro added the Simple Nice for a quick look on a minute or two label Dec 13, 2023
@cjdcordeiro cjdcordeiro requested a review from niemeyer December 13, 2023 09:41
@letFunny
Copy link
Collaborator Author

@cjdcordeiro You are absolutely right, I have added another field in the test to reflect that.

Copy link
Contributor

@niemeyer niemeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@niemeyer niemeyer merged commit 2266288 into canonical:main Dec 13, 2023
@letFunny letFunny deleted the ignore-yaml-extra-fields branch December 13, 2023 12:10
@cjdcordeiro
Copy link
Collaborator

@cjdcordeiro You are absolutely right, I have added another field in the test to reflect that.

A bit too late, but also not too urgent: @letFunny if you add a new key under slices, and it is not a map, chisel will fail to unmarshal it into setup.yamlSlice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Simple Nice for a quick look on a minute or two

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants