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

Add gettext as a required dependency #7

Closed
wants to merge 1 commit into from

Conversation

trarbr
Copy link

@trarbr trarbr commented Jan 1, 2022

This fixes compilation of Mix projects which have no explicit dependency on gettext.

I guess the dependency is no longer optional since gettext has been added to the list of compilers.

This fixes compilation of Mix projects which have no explicit dependency on gettext.
@kipcole9
Copy link
Collaborator

kipcole9 commented Jan 1, 2022

Thats very interesting, thanks for the report and PR.

I didn't expect that the project list of compilers would leak to an application using the library. Thats surprising. I have it in the ex_cldr_messages mix.exs for testing purposes.

My intention is that Gettext remain optional since not every use case requires it.

@trarbr
Copy link
Author

trarbr commented Jan 1, 2022

Oh okay 😄 I just pulled ex_cldr_messages down to play around with the library a bit and noticed that version 0.12 did not compile. After a bit of troubleshooting I found that adding gettext as a dependency of my (otherwise empty) Mix project made it work.

Should the gettext compiler only be added in the test environment then?

@kipcole9
Copy link
Collaborator

kipcole9 commented Jan 1, 2022

Thanks for confirming. I will do as you suggest and make sure that it doesn't require Gettext. Thanks for the collaboration.

@trarbr
Copy link
Author

trarbr commented Jan 1, 2022

No problem, happy to help - and thank you so much for your work on the various ex_cldr projects ❤️

@trarbr trarbr closed this Jan 1, 2022
@kipcole9
Copy link
Collaborator

kipcole9 commented Jan 2, 2022

I have published ex_cldr_messages version 0.13.0 with the following changelog entry:

Bug Fixes

  • Don't add :gettext to Mix.compilers/0 because it gets inherited into client applications and we want Gettext to remain optional. Thanks to @trarbr for the report.

  • Fix typos. Thanks as always to @kianmeng

Enhancements

  • Add an Elixir formatter plugin for sigil_M. For example in your .formatter.exs file:
[
  inputs: ["mix.exs", "{config,lib,test,mix}/**/*.{ex,exs}"],
  locals_without_parens: [docp: 1, defparsec: 2, defparsec: 3],
  plugins: [Cldr.Formatter.Plugin]
]

TIL that client apps inherit dependent mix compilers! Thanks for the report, the PR and the collaboration. I hope the experience is better now (although I am sure there are other issues waiting to be discovered).

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

2 participants