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

Provide a uniform api between Simple, KeyValue and Chain stores #109

Merged
merged 4 commits into from
Dec 11, 2018

Conversation

kidpollo
Copy link
Contributor

Provide a uniform api between Simple, KeyValue and Chain stores for public and private methods.

This is because a couple of gems I use out there need to call some of the internal methods of the simple store. When you try to use other gems configured with a Chain backend call those calls fail.

Basically I added the initialized?, translations and init_translations to the KeyValue and Chain stores.

Also added the functionality to be able to read back the translations from a key value store back to a hash as if it where read from the YAML files.

The gems that i know that would benefit from this would be i18n-js and babilu

@svenfuchs
Copy link
Collaborator

Whoops, commented over here kidpollo@3bc7ac9#commitcomment-601777 where I though I'd comment on the pull request. Maybe more coffee is in order.

So, I kinda like the idea.

I do think though that if we're supporting an api for 3rd party code then we should make it public.

Still pondering the exact api though.

@kidpollo
Copy link
Contributor Author

Examples of where protected instance methods are used elsewhere:

https://github.com/fnando/i18n-js/blob/master/lib/i18n-js.rb#L127

https://github.com/toretore/babilu/blob/master/lib/i18n_extensions.rb#L11

My particular use case is because I want to use either of these gems with a Redis Store chained to the Simple store and neither Chain or Key Stores have the appropriate methods to be used with these either of these gems

@kidpollo kidpollo closed this Sep 21, 2011
@kidpollo kidpollo reopened this Sep 21, 2011
@kidpollo
Copy link
Contributor Author

Woops closed this by accident :P

@robotmay
Copy link

Any further updates on this? Just bumped into the issue with i18n-js; I'm probably going to work around it for now but this would be useful in the future.

@kidpollo
Copy link
Contributor Author

I have not checked this out in a while. I will take a look at the most recent changes on the gem to see if things have changed.

Is there still an interest on providing uniform public apis for all backends?

@radar
Copy link
Collaborator

radar commented Nov 20, 2016

@kidpollo Yes I like this idea too. Could you please update your branch against the latest master? Will look at releasing this in 0.9.0.

@radar radar added this to the 0.9.0 milestone Nov 20, 2016
@kidpollo
Copy link
Contributor Author

Woah 5 years later still relevant?

@radar
Copy link
Collaborator

radar commented Nov 24, 2016

@kidpollo Do you think it would be relevant? Is it still useful to provide a uniform API?

@kidpollo kidpollo force-pushed the uniform_protected_api branch 2 times, most recently from 86ad57f to 0766d7b Compare November 24, 2016 08:52
@kidpollo
Copy link
Contributor Author

@radar updated

@radar radar modified the milestones: 0.9.0, 0.10.0 Oct 1, 2017
@dillonwelch
Copy link
Contributor

Still seems useful if you want to make another attempt at getting it cleaned up and merged.

@kidpollo
Copy link
Contributor Author

kidpollo commented Dec 5, 2018

Updated again

README.md Outdated Show resolved Hide resolved
lib/i18n/backend/chain.rb Outdated Show resolved Hide resolved
lib/i18n/backend/key_value.rb Outdated Show resolved Hide resolved
test/backend/chain_test.rb Outdated Show resolved Hide resolved
test/backend/chain_test.rb Outdated Show resolved Hide resolved
test/backend/key_value_test.rb Outdated Show resolved Hide resolved
@radar
Copy link
Collaborator

radar commented Dec 5, 2018

Thank you for keeping this continuously up to date. I have a few comments for you, and then I think this is good to merge :)

@kidpollo
Copy link
Contributor Author

kidpollo commented Dec 8, 2018

Done @radar

@radar
Copy link
Collaborator

radar commented Dec 8, 2018

Thank you!

I will merge this on Monday when I am back at work.

@radar radar merged commit f33374d into ruby-i18n:master Dec 11, 2018
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.

5 participants