-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat: add i18n support for default error messages #16
Conversation
That is some really great looking work so far. I'm glad you opened this cause I don't use gettext much. Just assign me for review whenever you're ready for me to formally review. Only request I have is that it would be nice if we could have some tests around this. |
@srcrip I'll add tests in the demo folder and I ping you when it is done |
@srcrip I added the test but I don't have permission to assign you as a reviewer. |
Wonderful! I'll take a look. |
@srcrip do you have any idea when you'll be able to review this PR? |
@srcrip I think I've found a way to simplify this PR a little bit. I'll play around a bit and try to push a new version in the next couple of days |
@srcrip just pushed a new version that is less intrusive and more straightforward to use. |
@srcrip, I'd appreciate some feedback on this PR. Specially if there is anything preventing you from merging it. Thanks in advance |
Thank you! Sorry for the delay, I've been working on a different feature that's taken up a lot of my time for the project. |
@srcrip I test locally with a project of mine and it works as expected.
Please let me know if you don't like the direction of the solution and I can try to explore other ideas.
closes #13