Skip to content

Conversation

@ecrupper
Copy link
Contributor

Actually setting the metadata fields when a secret is updated. I also made the existing test actually test for an update so this hopefully won't happen in the future.

@ecrupper ecrupper added the bug Indicates a bug label Jan 27, 2022
@ecrupper ecrupper self-assigned this Jan 27, 2022
@ecrupper ecrupper requested a review from a team as a code owner January 27, 2022 16:48
@codecov
Copy link

codecov bot commented Jan 27, 2022

Codecov Report

Merging #578 (d942ef1) into master (f40a1c9) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #578      +/-   ##
==========================================
+ Coverage   54.43%   54.44%   +0.01%     
==========================================
  Files         181      181              
  Lines       15131    15136       +5     
==========================================
+ Hits         8236     8241       +5     
  Misses       6578     6578              
  Partials      317      317              
Impacted Files Coverage Δ
secret/native/update.go 79.54% <100.00%> (+2.62%) ⬆️

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

FWIW:

In the future, we should consider removing this code in the secret/native/update.go file 😅

It feels like we're already handling this logic in the API handler:

https://github.com/go-vela/server/blob/master/api/secret.go#L546-L547

@ecrupper ecrupper merged commit eb7f283 into master Jan 27, 2022
@ecrupper ecrupper deleted the fix/secret-metadata-update branch January 27, 2022 17:05
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.

4 participants