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

Support ReplaceOp in Signatures #3315

Merged
merged 1 commit into from
Oct 29, 2023

Conversation

jonjohnsonjr
Copy link
Contributor

Before this change, calling WithReplaceOp didn't do anything for signatures and only affected attestations. I believe this is because the implementation of Dedupe and Replace was copied to 6 different methods, so they weren't getting updated.

I've extracted the core of the impl into a dedupeAndReplace method on opts, which makes each impl just a one line call with either signatures or attestations passed in.

Before this change, calling WithReplaceOp didn't do anything for
signatures and only affected attestations. I believe this is because the
implementation of Dedupe and Replace was copied to 6 different methods,
so they weren't getting updated.

I've extracted the core of the impl into a dedupeAndReplace method on
opts, which makes each impl just a one line call with either signatures
or attestations passed in.

Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
@codecov
Copy link

codecov bot commented Oct 21, 2023

Codecov Report

Merging #3315 (cb5db28) into main (2a33555) will decrease coverage by 0.35%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3315      +/-   ##
==========================================
- Coverage   30.65%   30.31%   -0.35%     
==========================================
  Files         155      155              
  Lines        9994     9919      -75     
==========================================
- Hits         3064     3007      -57     
+ Misses       6474     6462      -12     
+ Partials      456      450       -6     
Files Coverage Δ
pkg/oci/mutate/mutate.go 91.37% <100.00%> (+4.23%) ⬆️

Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

lgtm

@mattmoor mattmoor merged commit 9400476 into sigstore:main Oct 29, 2023
28 checks passed
@github-actions github-actions bot added this to the v2.3.0 milestone Oct 29, 2023
@jonjohnsonjr jonjohnsonjr deleted the replace-op-in-sigs branch October 29, 2023 01:26
@cpanato cpanato modified the milestones: v2.3.0, v2.2.1 Nov 16, 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.

3 participants