-
Notifications
You must be signed in to change notification settings - Fork 256
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
Be More Dry #134
Be More Dry #134
Conversation
}); | ||
}; | ||
|
||
player.on(videoEvents, function(event) { |
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 you still name this function redispatch
?
87241ea
to
513966f
Compare
}); | ||
}; | ||
|
||
player.on(videoEvents, function redispatch(event) { |
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 assume the redispatch function is the same just moved? The diff makes it a bit hard to see it fully/exactly.
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.
Yeah, it got moved to be an inline callback and the construction of the videoEvents
was greatly simplified.
513966f
to
be236ee
Compare
register(obj, events[i], handler); | ||
} | ||
break; | ||
case '[object Object]': |
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.
were we ever even using this case in the code? This would be the only thing that possibly could break.
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.
No, I checked - only strings and arrays!
Adding |
Yeah, sorry about that. 😄 |
LGTM |
This dries up a lot of code that was old and no longer needed. It builds on #133.