-
Notifications
You must be signed in to change notification settings - Fork 65
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
The manifest_version should default to 1 if missing. #214
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.
Fantastic! Glad this got caught.
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.
Minor nit:
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.
a58de47
to
31f3849
Compare
Kailan, Patrick and myself have discussed the implementation internally and settled on a solution (which is now implemented). |
Problem: Originally if the manifest_version field was missing from the fastly.toml manifest file, then we would return an error.
Solution: We should instead set a default value of
1
.Notes: to issue an appropriate “warning” message, it required the manifest
File
object to have access to a consistent output stream. This meant everywhere we callc.manifest.File.Read(manifest.Filename)
(which is in every command) I’ve had to precede it with a call toc.manifest.File.SetOutput(c.Globals.Output)
.