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

Versioning/depency to carrierwave and updating translations for carrierwave 3 (#3 and #25) #27

Merged
merged 10 commits into from
Sep 18, 2023
Merged

Versioning/depency to carrierwave and updating translations for carrierwave 3 (#3 and #25) #27

merged 10 commits into from
Sep 18, 2023

Conversation

araccaine
Copy link
Contributor

@araccaine araccaine commented Sep 11, 2023

Summary

This PR addresses #3 and #25 as they are both strongly related.

Please feel free to comment, revise and update this PR.

What

Tests

Two tests failed when running $ rspec because the capitalization was wrong: translation instead of Translation. I updated these characters.

Versioning

  • Added carrierwave version 3 as runtime dependency and use 3.0.0 as new version for this gem as proposed in Versioning #3

Translations and keys

See #25

  • Updated the key names to reflect changes in carrierwave 3 (see Carrierwave 3.x changed and added some translation keys #25)
    • _whitelist_ to _allowlist_
    • _blacklist_ to _denylist_
  • Merged the _processing_error keys into a generic one and updated the translations if possible
    • Removed the part where the error is added to the translation (... %{e} ...)
    • Removed the parts with rmagick / imagemagick strings were I could safely do so. Please note: Japanese ja and Thai th were not updated because I couldn't update and verify these strings for correctness
  • Added translations for the new keys if possible
    • German (de) by myself
    • ❤️ to @Jasonyang1991 for the new Chinese translations (zh-TW and zh-CN) and for enhancing the existing ones

Further comments

One may also create a new branch like carrierwave-2-x from the current carrierwave-i18n version 0.2.0 and update the gemspec file there as well for consistency? Maybe also updating the version number to 2.2.0 to be consistent?

Maybe also the pipeline for publishing on rubygems.org needs an update to allow independent managing of this gem?
E.g. the updated chinese translations may also go to version 0.2.0 e.g. as new "carrierwave 2 dependent version" 0.2.1.

@mshibuya
Copy link
Member

How wonderful, thank you so much for working on such a heavy lifting!
I'll prepare a gem release for CarrierWave pre-3.x versions, please wait for a moment.

rails/locales/ja.yml Outdated Show resolved Hide resolved
rails/locales/th.yml Outdated Show resolved Hide resolved
@araccaine
Copy link
Contributor Author

How wonderful, thank you so much for working on such a heavy lifting! I'll prepare a gem release for CarrierWave pre-3.x versions, please wait for a moment.

Thank you, too :)

Regarding the merge conflict in lib/carrierwave-i18n/version.rb:
I rebased my branch onto your latest changes and updated the Korean locale as well. Just to be clear:

  • Version 0.3.0 is the updated gem version for Carrierwave 1 and 2
  • Version 3.0.0 is the gem version for Carrierwave 3

Did I got this right? Otherwise feel free to correct this :)

@araccaine
Copy link
Contributor Author

I think you need at least Ruby 2.5 for the automated test with Ruby 2:

Bundler could not find compatible versions for gem "ruby": 
  In Gemfile: 
    ruby 
      carrierwave-i18n was resolved to 3.0.0, which depends on 
        carrierwave (< 4, >= 3.0.0) was resolved to 3.0.3, which depends on
          ruby (>= 2.5.0)

Carrierwave 3 depends on Ruby >= 2.5.0
@mshibuya
Copy link
Member

Version 0.3.0 is the updated gem version for Carrierwave 1 and 2
Version 3.0.0 is the gem version for Carrierwave 3

That's correct! I think it's good to go now.
Thank you for the amazing contribution 🌟

@mshibuya mshibuya merged commit f027c08 into carrierwaveuploader:master Sep 18, 2023
2 checks passed
@mshibuya
Copy link
Member

@araccaine Just released 3.0.0 🚀
By the way, are you interested in having the committer access to this repo? I think you now know this project very well...

@araccaine
Copy link
Contributor Author

@araccaine Just released 3.0.0 🚀

Nice 🥳

By the way, are you interested in having the committer access to this repo? I think you now know this project very well...

Yes I would be glad to do so :)

@mshibuya
Copy link
Member

Great! The invitation is on the way 👍

@araccaine araccaine deleted the feat/issue-3-and-25_versioning_carrierwave-3-x branch September 19, 2023 08:12
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.

2 participants