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

Remove i18n-inflector-rails dependency #8420

Merged
merged 2 commits into from Jun 10, 2023

Conversation

SuperTux88
Copy link
Member

There are no new releases anymore and the current version isn't compatible with ruby 3.x, see: siefca/i18n-inflector-rails#10 and siefca/i18n-inflector-rails#11

As this feature wasn't really used a lot (Icelandic didn't even setup inflections properly), it's probably not worth fighting for it, so lets just drop it.

Related to #8369

There are no new releases anymore and the current version isn't
compatible with ruby 3.x.

As this feature wasn't really used a lot (Icelandic didn't even setup
inflections properly), it's probably not worth fighting for it, so lets
just drop it.

Related to diaspora#8369
@SuperTux88 SuperTux88 added this to the 1.0.0 milestone Jun 8, 2023
@denschub
Copy link
Member

denschub commented Jun 9, 2023

I feel like leaving a comment here just for the after-world if this issue resolves itself. This is too funny to let go.

The tests for this PR are currently red:

Screenshot 2023-06-09 at 03 44 47

Why, you ask? Well:

Screenshot 2023-06-09 at 03 45 30

Because our test suite somehow decided that it now wants to run in French instead of English. Why does it do that? We have absolutely no idea.

@Flaburgan
Copy link
Member

Because our test suite somehow decided that it now wants to run in French instead of English. Why does it do that? We have absolutely no idea.

We run the world 😂

@SuperTux88 SuperTux88 force-pushed the remove-i18n-inflector-rails branch 3 times, most recently from 9ba4b85 to a8e3c64 Compare June 10, 2023 01:44
`deep_merge!` modifies the hash of the default translation, and it looks
like `I18n.t` always returns the same instance, so after that, the
default stays translated. So lets duplicate the hash first, before
modifying it, this also helps because we also add more keys below, which
probably also shouldn't be added to the original.
@SuperTux88
Copy link
Member Author

Because our test suite somehow decided that it now wants to run in French instead of English. Why does it do that? We have absolutely no idea.

OK, as fun as this sounds, it was actually very hard to debug and find out what's going on (especially because whatever I tried, I wasn't able to reproduce it locally). But I finally found the reason ... it turns out that after the locale was set to French once, it modified the English default translations, and after that point English was translated to French in-memory ... so lets just not do that anymore ...

I have no idea how exactly that worked before, maybe something with the inflector prevented that from happening? But I already debugged enough today, so I'm just happy it works now.

@SuperTux88 SuperTux88 merged commit d5a338a into diaspora:develop Jun 10, 2023
8 checks passed
@SuperTux88 SuperTux88 deleted the remove-i18n-inflector-rails branch June 10, 2023 15:28
@denschub
Copy link
Member

Amazing investigative work, @SuperTux88. 🙌

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.

None yet

5 participants