-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Deprecate Observable#throttle for Observable#debounce and RxJava-named methods #352
Conversation
@L8D Yes, you need to update all docs, tests and so forth for this to be accepted |
@headinthebox @benjchristensen can you weigh in here as to the correct semantics of debounce and the RxJava named throttle* methods? |
I have discussed this soo many times that my memory has gotten blurry. I do know that @benjchristensen thinks the .NET semantics are too UI specific and that we needed something else for the server side. I would say, switch to the names and semantics of RxJava https://github.com/ReactiveX/RxJava/wiki/Filtering-Observables. |
I would be concerned by the amount of breaking code this change would make. if you decide to go forward with the change, make sure to provide sufficient guidance to assist devs fixing their code that this change would cause. |
@jwooley we're deprecating, not removing, so you can get a warning and not an error. Just saying by the next major revision, they will likely be gone |
@mattpodwysocki Thanks for making this change, it really helps to clarify the language. Here is the original RxJava issue with links to definitions of Here is a snippet from it: Here is a good place to get a description of
Other links includes:
We have the following operators in RxJava:
For better or worse we use aliases to try and and make it clear that |
@benjchristensen @headinthebox sounds good. @L8D do you want me to make the doc changes for you as well as the tests? We should also add a "Deprecated APIs section as well as to not trip people up. |
Deprecate Observable#throttle for Observable#debounce and RxJava-named methods
@mattpodwysocki I can't tell if you made the changes for me atm, but if you did thanks. Otherwise, I'll be making another PR later today. |
@L8D yeah, I made the change, and I'm deprecating others while I'm at it. |
Is this good? The docs and perhaps the tests need to be updated.
Closes #284