Skip to content

Observable drop long#1098

Merged
Avasil merged 3 commits intomonix:masterfrom
gclaramunt:observable-drop-long
Jan 8, 2020
Merged

Observable drop long#1098
Avasil merged 3 commits intomonix:masterfrom
gclaramunt:observable-drop-long

Conversation

@gclaramunt
Copy link
Contributor

See #1072

@Avasil
Copy link
Collaborator

Avasil commented Dec 15, 2019

Thanks for PR @gclaramunt sorry for not following up on it earlier

Unfortunately the change is not binary compatible so we can't replace it until 4.x

I don't remember the issue about the overloads, I will try to dig it up

I suppose we are stuck with separate method name unless we can find a way to make it compatible

@gclaramunt
Copy link
Contributor Author

Ah, sorry I missed the binary check... I'll try to check the issue too, it might not be a big deal.
Otherwise, I'll change it to dropLong

@gclaramunt
Copy link
Contributor Author

Here are the issues with overloading: https://stackoverflow.com/questions/2510108/why-avoid-method-overloading but I've tested and it doesn't happen with Int and Long (it picks int).
Still, it might be risky ... let me know if you want to got with dropLong or something else

Copy link
Collaborator

@Avasil Avasil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's leave it like this and we will remove it in 4.x. I didn't figure out better way and it shouldn't be too problematic.

@Avasil Avasil merged commit dc31710 into monix:master Jan 8, 2020
@gclaramunt
Copy link
Contributor Author

cool, thanks! :)

mdedetrich pushed a commit to mdedetrich/monix that referenced this pull request Mar 28, 2020
* use long in drop instead of int

* follow contribution guidelines

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants