-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: add support for "v2-archives" in "v1" format #23
Conversation
"v2-archives" defines the archives, same as "archives". It is added to define Ubuntu Pro archives in chisel-releases with "pro" and "priority" fields (see canonical#160 and canonical#167), while supporting Chisel<=v1.0.0 and chisel-releases "format"<=v1. Since Chisel ignores unknown fields, archives defined in "v2-archives" will be ignored by v1.0.0 but picked up by later versions.
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.
Thanks for this Rafid! Almost ready, just added some light improvements.
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 overall good to me. Just one nitpick besides Alberto's reviews.
@@ -161,7 +172,7 @@ func parseRelease(baseDir, filePath string, data []byte) (*Release, error) { | |||
if yamlVar.Format != "v1" { | |||
return nil, fmt.Errorf("%s: unknown format %q", fileName, yamlVar.Format) | |||
} | |||
if len(yamlVar.Archives) == 0 { | |||
if len(yamlVar.Archives)+len(yamlVar.V2Archives) == 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.
The temporal variable len(yamlVar.Archives)+len(yamlVar.V2Archives)
is used twice, can we give it a name?
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 don't think it's that big of a deal to have a variable for this. Let me know if you have strong preferences for this.
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.
Two very very minor comments Rafid, thanks for this!
v2-archives
defines the archives, the same asarchives
. It is added to define Ubuntu Pro archives in chisel-releases withpro
andpriority
fields (see canonical#160 and canonical#167), while supporting Chisel<=v1.0.0 and chisel-releasesformat
<=v1. Since Chisel ignores unknown fields, archives defined inv2-archives
will be ignored by v1.0.0 but picked up by later versions.-- ROCKS-1729