-
Notifications
You must be signed in to change notification settings - Fork 84
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
Replace PO parse / compose with Expo library #307
Replace PO parse / compose with Expo library #307
Conversation
@whatyouhide I would really like to finish up this PR. Will you be able to have a look at the outstanding parser issue or should I try to find some help somewhere else? |
@maennchen hey, sorry for the wait. I was spending my OSS cycles mostly on Xandra the past few weeks, and I have just a little more to go. I would be able to take a look at this in something like a couple of weeks, would that works? If I'll get to it sooner, I'll get to it sooner 😄 |
@whatyouhide Thanks for the feedback. Xandra sounds interesting, I'll have to look at that one a bit closer :) I think I'm going to attempt it once more on my own in the meantime and if that doesn't work I'll wait for you. |
@whatyouhide I tried in a new way and I nearly got it working. To get to this point, I added a There are now only 2 errors left. It seems like the Details: elixir-gettext/expo#30 |
dca2257
to
62a1493
Compare
0d7a230
to
4805d5b
Compare
@josevalim FYI: I quickly rebased & fixed the tests. I will have to do one more pass over the PR and make sure all the naming etc. is correct. |
e131109
to
14f3bca
Compare
@josevalim / @whatyouhide I've completed the integration. A few points to note:
Thank you both for your time :) |
14f3bca
to
2a192cb
Compare
@@ -1,139 +1,176 @@ | |||
defmodule Gettext.MergerTest do | |||
defmodule GettextMergerTest do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defmodule GettextMergerTest do | |
defmodule Gettext.MergerTest do |
💚 💙 💜 💛 ❤️ |
@josevalim Oh wow, I was not expecting for this to get through that easy. Thanks ❤️ |
We just need an expo release and we can start working on the other issues. :) |
Partial implementation of #215
Blocked by elixir-gettext/expo#30
This is only an initial draft and not the finished implementation. I'll be on vacation until 25th and will probably not work on this until then.