Skip to content
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 slotOnload to use the correct on event name #75

Merged
merged 1 commit into from
Mar 13, 2018

Conversation

gabelew
Copy link
Contributor

@gabelew gabelew commented Mar 13, 2018

The event name returned is slotOnload, so the funcName generated in _onEvent in createManager.js is actually onSlotOnload and needs to be corrected.

@codecov
Copy link

codecov bot commented Mar 13, 2018

Codecov Report

Merging #75 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #75   +/-   ##
=======================================
  Coverage   93.96%   93.96%           
=======================================
  Files           8        8           
  Lines         580      580           
=======================================
  Hits          545      545           
  Misses         35       35
Impacted Files Coverage Δ
src/Bling.js 90.95% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08a9b1c...47fcc0e. Read the comment docs.

@potench
Copy link
Contributor

potench commented Mar 13, 2018

I don't think we have tests for any of these GPT pass-through events. Should consider introducing one here. Gonna accept this for now, but if you have a minute, can you PR a couple test cases to validate these pass-through events?

Copy link
Contributor

@potench potench left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, but would be nice to have a test to validate this.

@potench potench merged commit 2243755 into nfl:master Mar 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants