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

Listener Renaming: Rename updateListeners(Any) in ListenerManager #1574

Merged
merged 1 commit into from Jun 12, 2014

Conversation

Projects
None yet
2 participants
@Shadowfiend
Copy link
Member

Shadowfiend commented Jun 10, 2014

We do this because there is an updateListeners overload that takes a
list of actors to update, which is optional. Having an updateListeners
overload that takes an Any makes it really easy to accidentally call the
wrong updateListeners method by having the wrong type of List.
Additionally, it's particularly confusing to have the two methods named
the same thing when they behave differently.

We name the method that sends all listeners a message that is passed in
sendListenersMessage and leave updateListeners for sending all
listeners the message generated by createUpdate.

updateListeners(Any)->sendListenersMessage.
We do this because there is an updateListeners overload that takes a
list of actors to update, which is optional. Having an updateListeners
overload that takes an Any makes it really easy to accidentally call the
wrong updateListeners method by having the wrong type of List.
Additionally, it's particularly confusing to have the two methods named
the same thing when they behave differently.

We name the method that sends all listeners a message that is passed in
`sendListenersMessage` and leave `updateListeners` for sending all
listeners the message generated by `createUpdate`.
@fmpwizard

This comment has been minimized.

Copy link
Member

fmpwizard commented Jun 10, 2014

Don't we need a deprecate in 2.6 to go along with this?

@Shadowfiend

This comment has been minimized.

Copy link
Member Author

Shadowfiend commented Jun 10, 2014

Good catch; will issue a PR for that tomorrow.

@fmpwizard

This comment has been minimized.

Copy link
Member

fmpwizard commented Jun 10, 2014

cool, with that, 👍

@Shadowfiend

This comment has been minimized.

Copy link
Member Author

Shadowfiend commented Jun 12, 2014

Sending 'er in.

Shadowfiend added a commit that referenced this pull request Jun 12, 2014

Merge pull request #1574 from lift/listener-renaming
Listener Renaming: Rename updateListeners(Any) in ListenerManager

@Shadowfiend Shadowfiend merged commit 56fcdb1 into lift_30 Jun 12, 2014

@Shadowfiend Shadowfiend deleted the listener-renaming branch Jun 12, 2014

@Shadowfiend Shadowfiend added this to the 3.0-M2 milestone Aug 3, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment