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

Update getExecutingScript to fix issue where "cnvr" labels are hardcoded into code #17

Open
jeffreytgilbert opened this issue Jul 5, 2017 · 4 comments
Assignees
Labels

Comments

@jeffreytgilbert
Copy link
Contributor

https://github.com/conversant/ad-libs.js/blob/master/lib/dom/getExecutingScript.js

getExecutingScript has default flags set to options specific to an implementation in an external script, which is bad practice and could result in odd errors in other scripts using this functionality. Please remove this default label data and if that functionality is required, move those behaviors to params. It should not be required behavior. This is used in multiple places already and only one i know of requires this behavior.

@j-brown
Copy link
Collaborator

j-brown commented Jul 5, 2017

@jeffreytgilbert @msahagu2

This is a breaking change. We'll be moving to 3.0.0 with this update.

@jeffreytgilbert
Copy link
Contributor Author

WFM.

@jeffreytgilbert
Copy link
Contributor Author

@j-brown does that mean the external scripts using this functionality need to be updated as well? If so, can we open some JIRAs for those requests?

@j-brown
Copy link
Collaborator

j-brown commented Jul 6, 2017

@jeffreytgilbert Correct. We'll see breaking changes across the board.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants