Skip to content

fix: deprecate "chisel-v1" format support - #16

Merged
letFunny merged 5 commits into
letFunny:deprecate-chisel-v1from
rebornplusplus:fix/deprecate-chisel-v1
Jul 29, 2024
Merged

letFunny merged 5 commits into
letFunny:deprecate-chisel-v1from
rebornplusplus:fix/deprecate-chisel-v1

Conversation

@rebornplusplus

@rebornplusplus rebornplusplus commented Jul 4, 2024 •

Copy link
Copy Markdown

This PR deprecates 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 older version which does not have this commit.

@rebornplusplus

Copy link
Copy Markdown
Author

Raised in anticipation of the merging of the DB PRs. Not urgent. 🟢

@rebornplusplus
rebornplusplus force-pushed the fix/deprecate-chisel-v1 branch from 763bea0 to e0fc0f5 Compare July 4, 2024 14:02
@rebornplusplus rebornplusplus changed the title fix: deprecate "chisel-v1" format support fix: depreciate "chisel-v1" format support Jul 4, 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 changed the base branch from main to verify-release-bump-version July 8, 2024 07:42
@letFunny
letFunny changed the base branch from verify-release-bump-version to main July 8, 2024 07:43

@letFunny letFunny left a comment

Copy link
Copy Markdown
Owner

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".

Comment thread internal/setup/setup_test.go Resolved
Comment thread internal/setup/setup_test.go Resolved
@rebornplusplus rebornplusplus changed the title fix: depreciate "chisel-v1" format support fix: deprecate "chisel-v1" format support Jul 26, 2024
@rebornplusplus
rebornplusplus requested a review from letFunny July 26, 2024 11:10

@letFunny letFunny left a comment

Copy link
Copy Markdown
Owner

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

Comment thread internal/setup/setup_test.go Outdated
v1-public-keys: [extra-key]
default: true
v1-public-keys:
extra-key:

Copy link
Copy Markdown
Owner

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Removed it in 18e61a7.

Copy link
Copy Markdown
Owner

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sure.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sure. Added in b869141.

Comment thread internal/setup/setup_test.go Resolved
Comment thread internal/slicer/slicer_test.go Resolved
@letFunny
letFunny changed the base branch from main to deprecate-chisel-v1 July 29, 2024 10:18
@letFunny
letFunny merged commit c7b49ba into letFunny:deprecate-chisel-v1 Jul 29, 2024
letFunny pushed a commit that referenced this pull request 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants