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

Documentation question #404

Closed
BradS2S opened this issue Dec 5, 2024 · 6 comments
Closed

Documentation question #404

BradS2S opened this issue Dec 5, 2024 · 6 comments
Assignees

Comments

@BradS2S
Copy link
Contributor

BradS2S commented Dec 5, 2024

Examples use the format: MyApp.Gettext

Since Phoenix generator automatically puts it here: MyAppWeb.Gettext, does it make sense to make that documentation change throughout?

@whatyouhide
Copy link
Contributor

No, it does not. Gettext is not tied to Phoenix in any way in and of itself, it's Phoenix which depends on Gettext. To avoid confusion, we can rename MyApp to something else but we shouldn't mention "web". Is this actually confusing though?

@BradS2S
Copy link
Contributor Author

BradS2S commented Dec 6, 2024

That's what I figured. It makes sense that the documentation can't be tightly coupled with just one framework.

Here's what I ran across specifically that I suspect will grow more common with the deprecation warning:

If you happen to be using Phoenix, and you change

use Gettext, otp_app: ...

to

use Gettext.Backend, otp_app: :my_app,

you also have to change import MyAppWeb.Gettext in lib/my_app_web/components/core_components.ex.

Since the deprecation warming says:

Then, instead of importing your backend, call this in your module:

    use Gettext, backend: MyApp.Gettext

I was copying it from the warning code and then just changing the "MyApp" part and leaving the "Web" omitted because I wasn't super confident I knew exactly what the change did so it took me a sec to realize my mistake. Not a big deal I just wanted to mention it because I ran across someone else that might have also been confused.

I'm wildly unfamiliar with what is best practices so I don't know if a note in the warning that mentions if you are using the Phoenix framework, you also need to change that other line in core_components.ex ,or it could be that a more helpful compile time error could be implemented in Phoenix since Gettext isn't tied to Phoenix.

Either way I found it interesting.

@whatyouhide
Copy link
Contributor

We can improve the warning (in Gettext) to use the actual module name, which would fix this issue. How does that sound? Want to give it a try?

@maennchen
Copy link
Member

If we’re doing that, we should also use the correct otp app instead of my_app.

@BradS2S
Copy link
Contributor Author

BradS2S commented Dec 6, 2024 via email

@BradS2S
Copy link
Contributor Author

BradS2S commented Dec 7, 2024

Here's the pull request -> #406

@BradS2S BradS2S closed this as completed Dec 7, 2024
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

No branches or pull requests

3 participants