-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Defend workflows] Replace differenceWith with xorWith and remove the helper #157966
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.
@tomsonpl it's a good simplification 👍 Don't forget to add the PR description.
@@ -95,9 +96,10 @@ export const validateResponseActionsPermissions = async ( | |||
|
|||
const endpointAuthz = await securitySolution.getEndpointAuthz(); | |||
|
|||
const differences = findDifferenceInArrays<ResponseAction, RuleResponseAction>( | |||
const differences = xorWith<ResponseAction | RuleResponseAction>( |
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.
Can it infer the type ResponseAction | RuleResponseAction
without explicitly providing 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 think it would infer the type if two arrays were of the same types, but in this case I had to specify these.
@@ -95,9 +96,10 @@ export const validateResponseActionsPermissions = async ( | |||
|
|||
const endpointAuthz = await securitySolution.getEndpointAuthz(); | |||
|
|||
const differences = findDifferenceInArrays<ResponseAction, RuleResponseAction>( | |||
const differences = xorWith<ResponseAction | RuleResponseAction>( |
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 like this better indeed. I was going to suggest xorWith
earlier but didn't want to ask you to change too much. 🎉 Thanks for updating this. I think you should name the varaible symmetricDifference
anyway. Maybe, also leave a comment above that says finds elements that are not in either array
, so one doesn't have to look up the function to understand what this does.
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.
That was a very good point, thank you @ashokaditya :)
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.
🚀 🚢
💚 Build Succeeded
Metrics [docs]Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @tomsonpl |
While using differenceWith we had to manually check for the empty array, otherwise it didn't return the difference. xorWith does this for us while returning the symmetric difference.