-
Notifications
You must be signed in to change notification settings - Fork 85
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
Fix process ID not being reset after event is emitted #2360
Conversation
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2360 +/- ##
========================================
Coverage 91.27% 91.27%
========================================
Files 636 636
Lines 18068 18071 +3
Branches 3895 3787 -108
========================================
+ Hits 16491 16494 +3
Misses 1576 1576
Partials 1 1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
let shouldProcessEvent = eeInst.eventTimes.get(eventName) !== event.eventTime; | ||
eeInst.eventTimes.set(eventName, event.eventTime); | ||
// Checks that event was not triggered by the same process | ||
shouldProcessEvent &&= process.pid !== event.appProcId; |
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 lines 208 and 211 be combined? Combining the lines will also allow you to use const
instead of let
:
const shouldProcessEvent = eeInst.eventTimes.get(eventName) !== event.eventTime && process.pid !== event.appProcId;`
Quality Gate passedIssues Measures |
Release succeeded for the The following packages have been published:
Powered by Octorelease 🚀 |
What It Does
This fixes a regression of #2323 where after the
onVaultChanged
event was ignored because of same process ID, then subsequentonVaultChanged
events from external apps would be ignored.Also updates the
cross-spawn
package for technical currency.How to Test
Review Checklist
I certify that I have:
Additional Comments