-
Notifications
You must be signed in to change notification settings - Fork 11
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
[FIX] GoogleTranslate errors #6
[FIX] GoogleTranslate errors #6
Conversation
@@ -1,5 +1,6 @@ | |||
module RailsI18nManager | |||
module GoogleTranslate | |||
require 'easy_translate' |
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.
According to general ruby standards, require
statements should be at the top of the file not within a class or method.
But I would prefer we do all of our dependency require statements in lib/rails_i18n_manager/engine.rb
@@ -13,7 +14,7 @@ def self.translate(text, from:, to:) | |||
return nil | |||
end | |||
|
|||
EasyTranslate.translate(text, from: from, to: to, key: api_key) | |||
translation = EasyTranslate.translate(text, from: from, to: to, key: api_key) |
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.
This looks fine for this PR. Though I wonder why we have some complexity in the statements below. They probably need to be refractored a bit.
if translation
str = translation.to_s
if str.present?
I've made one comment to address. Would you also mind adding a changelog entry? |
Looks like we should add at least one passing spec for this class |
Thanks for these fixes however I've added a more complete solution including adding specs for this class. See #11 |
Description
This PR addresses an issue with the Google Translate functionality in
RailsI18nManager::GoogleTranslate
module. The following fixes have been implemented:require 'easy_translate'
statement to ensure proper dependency utilization.This fix ensures that the Google Translate functionality now operates as intended, allowing for accurate translations and resolving any previous errors.