Skip to content

Conversation

@letFunny
Copy link
Collaborator

@letFunny letFunny commented Jul 29, 2024

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.


  • Have you signed the CLA?

@letFunny letFunny added the Simple Nice for a quick look on a minute or two label Jul 29, 2024
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.
@letFunny letFunny force-pushed the deprecate-chisel-v1 branch from c7b49ba to 322d890 Compare July 29, 2024 12:06
@letFunny letFunny added the Priority Look at me first label Oct 3, 2024
@letFunny letFunny requested a review from niemeyer October 3, 2024 10:36
armor: |` + "\n" + testutil.PrefixEachLine(testKey.PubKeyArmor, "\t\t\t\t\t\t") + `
`,
},
relerror: `chisel.yaml: unknown format "chisel-v1"`,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

[Question for reviewer]: The only question is whether we should introduce a better error message for chisel-v1 that says something like "consider an update if available" just like we do for invalid generate: values.

Copy link
Contributor

Choose a reason for hiding this comment

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

Per the conversation we had, to introduce such a message we need to do that at the very top layer, in the user interface itself (CLI, or GUI) as otherwise we cannot tell the context in which the library/package is being used. Even there, sometimes it may be misleading as the tool may be leveraged in servers and the error reported in a way that is impossible for the receiver of the message to act on the suggestion. For that kind of reason, it is usually better to report the error with a good error message which allows the reader to grasp what is going on by themselves, which seems like the case here already.

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.

Looks good, thank you.

@niemeyer niemeyer merged commit 944ee7c into canonical:main Oct 7, 2024
@letFunny letFunny deleted the deprecate-chisel-v1 branch October 17, 2024 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority Look at me first Simple Nice for a quick look on a minute or two

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants