-
Notifications
You must be signed in to change notification settings - Fork 39
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
update to use email from propagated claims #514
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. But I wonder why there is no changes in unit tests needed...
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #514 +/- ##
=======================================
Coverage 82.54% 82.54%
=======================================
Files 29 29
Lines 3220 3220
=======================================
Hits 2658 2658
Misses 426 426
Partials 136 136
|
if email := getMUR.Annotations[toolchainv1alpha1.MasterUserRecordEmailAnnotationKey]; email != "" { | ||
emails = append(emails, getMUR.Annotations[toolchainv1alpha1.MasterUserRecordEmailAnnotationKey]) | ||
if email := getMUR.Spec.PropagatedClaims.Email; email != "" { | ||
emails = append(emails, getMUR.Spec.PropagatedClaims.Email) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since you already assigned getMUR.Spec.PropagatedClaims.Email
into email
, you could do something like this:
emails = append(emails, getMUR.Spec.PropagatedClaims.Email) | |
emails = append(emails, email) |
but it's a minor thing
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbryzak, xcoulon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@alexeykazakov unit tests were updated shortly after :) |
Related PR: codeready-toolchain/host-operator#943