Skip to content

Conversation

@eksperimental
Copy link
Contributor

They are not needed, since they are covered by their /2 version.

They are not needed, since they are covered by their /2 version.
@josevalim josevalim merged commit 2622fd6 into elixir-lang:master Feb 7, 2019
@josevalim
Copy link
Member

❤️ 💚 💙 💛 💜

@eksperimental eksperimental deleted the remove_unneeded_public_functions branch February 7, 2019 13:15
michalmuskala added a commit that referenced this pull request Feb 7, 2019
@michalmuskala
Copy link
Member

@eksperimental did you benchmark the change? integer_to_binary/1 is a BIF, while integer_to_binary/2 is implemented in Erlang. I expect the performance difference to be considerable.

@ericmj
Copy link
Member

ericmj commented Apr 2, 2019

They seem to all be BIFs since OTP 21.3.

@michalmuskala
Copy link
Member

Ah, you're right. That still leaves quite a lot of supported versions with a potential performance degradation.

@eksperimental
Copy link
Contributor Author

I guess we should revert the PR and add annote to reapply it when 22 is the minimum supported version

eksperimental added a commit to eksperimental-forks/elixir that referenced this pull request May 15, 2019
@eksperimental eksperimental mentioned this pull request May 15, 2019
josevalim pushed a commit that referenced this pull request May 15, 2019
Revert "Remove Integer.to_string/1 and Integer.to_charlist/1 (#8777)"

This reverts commit 2622fd6.
@eksperimental
Copy link
Contributor Author

Fixed

josevalim pushed a commit that referenced this pull request Mar 29, 2021
They are not needed, since they are covered by their /2 version.
josevalim pushed a commit that referenced this pull request May 6, 2021
They are not needed, since they are covered by their /2 version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants