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

fix(secret)!: remove implicit repo secrets and clean up secrets testing #230

Merged
merged 4 commits into from
Mar 1, 2022

Conversation

KellyMerrick
Copy link
Contributor

This pr is to create a net-new pr for #210, which was reverted in #229. That pr was for a breaking change, that we decided not to introduce into an RC to be cut today.

@KellyMerrick KellyMerrick requested a review from a team as a code owner January 24, 2022 16:47
@KellyMerrick KellyMerrick self-assigned this Jan 24, 2022
@codecov
Copy link

codecov bot commented Jan 24, 2022

Codecov Report

Merging #230 (5671fda) into master (0ee0f1e) will increase coverage by 0.11%.
The diff coverage is 97.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #230      +/-   ##
==========================================
+ Coverage   97.06%   97.18%   +0.11%     
==========================================
  Files          53       53              
  Lines        5797     5818      +21     
==========================================
+ Hits         5627     5654      +27     
+ Misses        125      121       -4     
+ Partials       45       43       -2     
Impacted Files Coverage Δ
pipeline/secret.go 97.24% <97.77%> (+7.47%) ⬆️

Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

This pr is to create a net-new pr for #210, which was reverted in #229. That pr was for a breaking change, that we decided not to introduce into an RC to be cut today.

Could you elaborate on why that PR was reverted?

I think updating the description for #229 with more details would be ideal

@wass3r
Copy link
Collaborator

wass3r commented Jan 26, 2022

It was an accidental merge. Just prior to it we decided to freeze current state across core repos for the RC unless they were dependency upgrades.

@wass3r wass3r changed the title fix(secret)remove implicit repo secrets and clean up secrets testing fix(secret): remove implicit repo secrets and clean up secrets testing Feb 9, 2022
jbrockopp
jbrockopp previously approved these changes Feb 17, 2022
Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

LGTM

@wass3r wass3r changed the title fix(secret): remove implicit repo secrets and clean up secrets testing fix(secret)!: remove implicit repo secrets and clean up secrets testing Feb 17, 2022
Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

LGTM

@jbrockopp jbrockopp added the bug Indicates a bug label Feb 21, 2022
@wass3r
Copy link
Collaborator

wass3r commented Feb 24, 2022

@KellyMerrick anything else needed here?

@KellyMerrick KellyMerrick merged commit adee130 into master Mar 1, 2022
@KellyMerrick KellyMerrick deleted the dep-impl-secret2 branch March 1, 2022 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants