fix: [botframework-connector] Use HashSet instead of string array for endorsement #4526
Conversation
|
@microsoft-github-policy-service agree |
| @@ -25,14 +25,14 @@ export class EndorsementsValidator { | |||
| * some specific channels. That list is the endorsement list, and is validated here against the channelId. | |||
| * @returns {boolean} True is the channelId is found in the Endorsement set. False if the channelId is not found. | |||
| */ | |||
| static validate(channelId: string, endorsements: string[]): boolean { | |||
| static validate(channelId: string, endorsements: Set<string>): boolean { | |||
There was a problem hiding this comment.
Changing the function's signature would break the backward compat. Instead, could you create a new function that receives the Set argument and call it inside the old validate function?
There was a problem hiding this comment.
I'm not opposed to breaking this if having another method is clutter. Is there harm in having two?
There was a problem hiding this comment.
I suggest a private function to be called inside this old one (passing the converted strings[] as set) so those who are using this with an array won't experience a break.
There was a problem hiding this comment.
Sounds good, probably with a comment about it. Future refactor I suppose if we ever do another major release. v5.
Pull Request Test Coverage Report for Build 7522268835
💛 - Coveralls |
|
@ceciliaavila When you get a chance can you please run |
Fixes #4454
#minor
Description
This small PR just changes an argument type in the static validate method of the EndorsementsValidator class.
Specific Changes
Testing
All unit tests passed
