-
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
fix: deprecate "chisel-v1" format support #16
fix: deprecate "chisel-v1" format support #16
Conversation
Raised in anticipation of the merging of the DB PRs. Not urgent. 🟢 |
763bea0
to
e0fc0f5
Compare
This commit depreciates support for the "chisel-v1" format in chisel. Notably the following fields/values are no longer supported: - The "chisel-v1" value for top-level field "format". Use "v1" instead. - The "<archive>.v1-public-keys" field. Use "<archive>.public-keys" instead. - The top-level "v1-public-keys" field. Use "public-keys" instead. BREAKING CHANGE: New versions of chisel which includes this commit will no longer support the "chisel-v1" format in chisel-releases. Either update the "chisel.yaml" file in chisel-releases or use a lower version which does not have this commit.
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 Rafid, the PR is looking good, only some minor comments. Also, the commit message has a typo, "depreciate" -> "deprecate".
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, only a couple of nits
internal/setup/setup_test.go
Outdated
v1-public-keys: [extra-key] | ||
default: true | ||
v1-public-keys: | ||
extra-key: |
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.
Does this key have any significance? extra-key
seems to imply that it is different from a regular one.
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.
Removed it in 18e61a7.
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.
Can you add the v1-public-keys
back? I only meant using the default key instead of implying that this is an extra one. I think having a complete example of chisel-v1
is still useful for reference.
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.
Sure.
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.
Sure. Added in b869141.
c7b49ba
into
letFunny:deprecate-chisel-v1
This commit depreciates support for the "chisel-v1" format in chisel. Notably the following fields/values are no longer supported: - The "chisel-v1" value for top-level field "format". Use "v1" instead. - The "<archive>.v1-public-keys" field. Use "<archive>.public-keys" instead. - The top-level "v1-public-keys" field. Use "public-keys" instead. BREAKING CHANGE: New versions of chisel which includes this commit will no longer support the "chisel-v1" format in chisel-releases. Either update the "chisel.yaml" file in chisel-releases or use a lower version which does not have this commit.
This PR deprecates support for the "chisel-v1" format in chisel. Notably, the following fields/values are no longer supported:
chisel-v1
value for top-level fieldformat
. Usev1
instead.<archive>.v1-public-keys
field. Use<archive>.public-keys
instead.v1-public-keys
field. Usepublic-keys
instead.BREAKING CHANGE: New versions of chisel which includes this commit will no longer support the "chisel-v1" format in chisel-releases. Either update the "chisel.yaml" file in chisel-releases or use a older version which does not have this commit.