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

Blockly.bindEvent_ is deprecated, remove it's use internally #3332

Closed
samelhusseini opened this issue Oct 24, 2019 · 6 comments
Closed

Blockly.bindEvent_ is deprecated, remove it's use internally #3332

samelhusseini opened this issue Oct 24, 2019 · 6 comments
Assignees
Labels
internal External contributions not accepted

Comments

@samelhusseini
Copy link
Contributor

samelhusseini commented Oct 24, 2019

Filing this issue to track removing calls to Blockly.bindEvent_ internally as it is deprecated.
When removing these calls, we should also remove the @suppress {deprecated} annotations so the compiler doesn't skip those.

@samelhusseini samelhusseini added the internal External contributions not accepted label Oct 24, 2019
@BeksOmega
Copy link
Collaborator

I think there might be times when bindEvent_ needs to be called. See here and here. But I'm not sure.

@NeilFraser
Copy link
Member

FYI, it's called in 27 places in Blockly and 9 places by Blockly Games. Last time I checked there were no alternatives to using it in some circumstances. Not sure why it would be labeled as @deprecated.

@samelhusseini
Copy link
Contributor Author

Yeah I agree with Neil, if we're using it and want to keep it around we shouldn't mark as deprecated.

@samelhusseini
Copy link
Contributor Author

This PR is pretty much just ignoring the deprecated call, which is a round about way of saying we want to keep it "for now".

@rachel-fenichel
Copy link
Collaborator

rachel-fenichel commented Oct 24, 2019

We can remove the deprecation marker. bindEventWithChecks just doesn't fully replace it. However, uses of it in developer code may be a code smell.

For anything in the normal gesture workflow, bindEventWithChecks is correct. But anything that's not part of a gesture doesn't need it.

@RoboErikG RoboErikG added this to the Bug Bash Backlog milestone Oct 25, 2019
@samelhusseini samelhusseini self-assigned this Oct 30, 2019
@samelhusseini
Copy link
Contributor Author

Fixed.

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

No branches or pull requests

5 participants