-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
DRY up pkg/config envelope creation #1098
Conversation
/ci-run |
AZP build triggered! |
pkg/config/config.go
Outdated
// cb.ConfigGroupEnvelope proto message. | ||
// newEnvelope creates an unsigned envelope of the desired type containing | ||
// a payload Header and the marshaled proto message as the payload Data. | ||
// message |
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.
Did you forget to delete this line?
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.
Ah yep, didn't even notice it for a while after reading your comment. :) Removed.
/ci-run |
AZP build triggered! |
FAB-17741 Signed-off-by: Will Lahti <wtlahti@us.ibm.com>
|
||
signatureHeader, err := s.signatureHeader() | ||
// newSignedEnvelope creates a signed envelope of the desired type and signs it. | ||
func (s *SigningIdentity) newSignedEnvelope(txType cb.HeaderType, channelID string, dataMsg proto.Message) (*cb.Envelope, error) { |
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.
could maybe even call this just signedEnvelope
and just drop the new
since that seems to be the pattern we're moving towards
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.
I'm indifferent either way. I'll let you decide which you prefer, let me know if you want to merge as is or if you want to change it
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.
I say we keep it as-is for now. We can do another pass to make sure we're consistent in our naming of functions after the current set of tasks are merged in.
Type of change
Description
Reuse newEnvelope() when creating a signed envelope.
Related issues
FAB-17741