ci: Add Reusable PR Workflow #13
Conversation
| // EventName: "Error" | ||
| // } | ||
| }; | ||
| EventHandler.prototype.logPageView = function(event) { |
There was a problem hiding this comment.
i believe this will cause an error in the wrapper. https://github.com/mparticle-integrations/mparticle-javascript-web-kit-wrapper/blob/master/index.js#L160-L190
This doesn't check for the existence before calling.
So @SbDove - there are no page views in heap?
alexs-mparticle
force-pushed
the
ci/SQDSDKS-6560-add-pr-workflow
branch
from
b05d76c to
c2691a4
Compare
| // EventName: "Error" | ||
| // } | ||
| }; | ||
| EventHandler.prototype.logPageView = function(event) { |
| appName, | ||
| customFlags, | ||
| clientId | ||
| common |
There was a problem hiding this comment.
i know aI'd either leave the remaining arguments here along with an eslint disable before it for unused arguments, or a comment saying what other arguments are available. The likeliest one that may be used in the future is customFlags. If someone in the future came back to look at this, it may be hard for them to know where that should come from without a comment or the argument here.
There was a problem hiding this comment.
I think a comment works best as I don't want someone to forget to turn off the eslint disable setting in the future. Some of this feedback indicates that we should take a look at the example repo and consider adding something like JSDocs to declare what parameters are available for derived kits instead of adding unnecessary variables that could cause some code bloat.
Summary
npm run lintcommand to package.json so we can also run the linterTesting Plan
Master Issue
Closes https://go.mparticle.com/work/SQDSDKS-6560