Skip to content
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

sign: Make SigningResult._to_bundle() public #765

Merged
merged 3 commits into from
Sep 11, 2023

Conversation

jku
Copy link
Member

@jku jku commented Sep 8, 2023

Summary

This enables signing applications to use the bundle format and seems in line with the other similar decisions:

  • The bundle is already part of the CLI interface
  • verify already contains similar public API (VerificationMaterials.from_bundle() is public)

Closes #763

Release Note

  • API addition: sign.SigningResult.to_bundle() allows signing applications to serialize to the bundle format that is already usable in verification with verify.VerificationMaterials.from_bundle()

Documentation

  • Method already has a docstring
  • A complete example of sign -> bundle serialization -> bundle deserialization -> verify could be useful but I did not add one here

Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

LGTM -- needs a rebase/merge and a CHANGELOG entry 🙂

jku added 3 commits September 11, 2023 09:57
This enables signing applications to use the bundle format and seems in
line with the other similar decisions:
* The bundle is already part of the CLI interface
* verify already contains similar public API
  (VerificationMaterials.from_bundle() is public)

Closes sigstore#763

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Mention that changelog entry goes to CHANGELOG.md, not the PR
description.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
@jku jku force-pushed the make-to-bundle-method-public branch from f5fc2c3 to a953c5b Compare September 11, 2023 07:14
@jku
Copy link
Member Author

jku commented Sep 11, 2023

LGTM -- needs a rebase/merge and a CHANGELOG entry 🙂

Done. Tweaked the PR template as well to make it clearer the release note goes to CHANGELOG.md.

@woodruffw
Copy link
Member

/gcbrun

@woodruffw woodruffw merged commit 68aa69a into sigstore:main Sep 11, 2023
21 checks passed
@woodruffw
Copy link
Member

Thanks @jku!

@di di mentioned this pull request Sep 11, 2023
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.

make SigningResult._to_bundle() public
2 participants