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

Resolve build warnings of deprecated String.strip/1 method #8

Merged
merged 1 commit into from
Jul 28, 2023

Conversation

theodowling
Copy link
Contributor

I am using your library as a dependency in a personal project, and thought of committing this change to resolve some build warnings.

==> mustache
Compiling 1 file (.ex)
warning: String.strip/1 is deprecated, use String.trim/1
  lib/mustache.ex:26

warning: String.strip/1 is deprecated, use String.trim/1
  lib/mustache.ex:23

warning: String.strip/1 is deprecated, use String.trim/1
  lib/mustache.ex:58

Generated mustache app

Feel free to ignore or merge at your will.

@schultyy
Copy link
Owner

schultyy commented Sep 4, 2017

@theodowling Hey 👋

Sorry for the long delay here, I was quite busy lately.

Thank you for the Pull Request. I've seen that the tests for Elixir 1.2 are failing right now. Can you have a look at that?

@theodowling
Copy link
Contributor Author

theodowling commented Sep 4, 2017

Hi @schultyy

It seems like String.trim/1 isn't available in Elixir v1.2, so if there is a need to continue support for Elixir v1.2 I do understand and I'm happy if you ignore this PR 😄.

Just don't want this to become an issue in the future as it seems like String.strip/1 will be removed completely in Elixir v2 - elixir/lib/string.ex#L827 - even thought I don't know when that is going to be released.

@schultyy
Copy link
Owner

Hmm that makes sense. I spend some time to think about this and I'd say let's move forward with this. Because this will break behavior for people with Elixir 1.2 or before, I'd say let's increase the major version and put a notice in the README about this. WDYT?

@thiamsantos
Copy link

Any news on this?

@jsmestad
Copy link

@schultyy probably OK to drop 1.2 support

@schultyy schultyy merged commit 6ea70af into schultyy:master Jul 28, 2023
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.

None yet

4 participants