Skip to content

Restore Globalize: Fix gem dependencies and sandbox#155

Open
fthobe wants to merge 7 commits intosolidusio-contrib:mainfrom
TriodecSolutions:fix-gem-dependencies-and-sandbox
Open

Restore Globalize: Fix gem dependencies and sandbox#155
fthobe wants to merge 7 commits intosolidusio-contrib:mainfrom
TriodecSolutions:fix-gem-dependencies-and-sandbox

Conversation

@fthobe
Copy link

@fthobe fthobe commented Apr 2, 2025

This PR has no functional changes and just updates the Gem Dependencies.

@jarednorman @tvdeyen to you want to merge this into main?

After this change [solidusio/solidus@16fcb10](solidusio/solidus@16fcb10)

The command `bundle exec rake` started failing with the following error:
```
An error occurred in a `before(:suite)` hook.
Failure/Error: //= link_directory ../javascripts .js

Sprockets::ArgumentError:
  link_directory argument must be a directory
```

- Ensure that the correct directory is specified in the manifest file to avoid the `Sprockets::ArgumentError`.
Update en.yml to display accurate resource names on the crumbs of globalize pages.
Added default value on `friendly_id_slugs` table
to prevent not null constraint violation while gem installation.
@tvdeyen
Copy link
Member

tvdeyen commented Apr 2, 2025

@fthobe sure. any idea why the specs are failing?

@fthobe
Copy link
Author

fthobe commented Apr 2, 2025

@tvdeyen because we are idiots. Let me check.

@fthobe
Copy link
Author

fthobe commented Apr 2, 2025

@JustShah I looked at the tests and they all look like backend failures. Let's look at them next week.

@fthobe
Copy link
Author

fthobe commented Apr 2, 2025

@jarednorman we are going to bring this into working state and switch to mobility afterwards. As you are already working on it we would appreciate if you have any additional comments.

@fthobe
Copy link
Author

fthobe commented Apr 2, 2025

@tvdeyen do you want to use the chance to update to github actions here as well?

Copy link
Member

@jarednorman jarednorman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good. I'm wondering about the "Update Sandbox script" commit. Was this just pulled in from a more up-to-date extension? Should we instead be running the solidus_dev_support upgrade?

@fthobe
Copy link
Author

fthobe commented Apr 2, 2025

This is looking good. I'm wondering about the "Update Sandbox script" commit. Was this just pulled in from a more up-to-date extension? Should we instead be running the solidus_dev_support upgrade?

@JustShah can reply to that.

@JustShah
Copy link

JustShah commented Apr 3, 2025

This is looking good. I'm wondering about the "Update Sandbox script" commit. Was this just pulled in from a more up-to-date extension? Should we instead be running the solidus_dev_support upgrade?

Hey @jarednorman , The "Update Sandbox script" commit was indeed pulled in from a more up to date extension. The previous version was outdated as it did not utilize the Starter Frontend and was installing Solidus Auth separately, which is no longer necessary.

To address these issues, the script has been revised by referencing the latest practices from Solidus Braintree and Solidus Subscription

@jarednorman
Copy link
Member

Does the extension need a solidus_dev_support upgrade?

@fthobe
Copy link
Author

fthobe commented Apr 3, 2025

Does the extension need a solidus_dev_support upgrade?

If not neccessary, I would prefer to kill this off and move to mobility. By account of the globalize maintainers, it is a dead man walking, mobility looks very backward compatible to me.

@jarednorman @tvdeyen @kennyadsl

Offering a totally not asked for 3rd party opinion something that should:

  1. migrated to mobility as by auto declaration globalize is not supported anymore while mobility (by the same maintainers) is;
  2. be merged with i18 (core extensions) as providing string translation but not content translation seems making everything more difficult;
  3. offered in a version individually and merged with static content to also allow pages to work.

On the risk of highjacking this, I would actually consider to make breaking change in the new migration. I am sure that mayur can get this over the finish line.

This would allow us to reduce the number of extensions from three (static / i18 / globalize) to two (multilanguage / multilanguage and content).

@fthobe
Copy link
Author

fthobe commented Apr 3, 2025

@JustShah attribution must be given, please provide the link to the repos which you have used.

@kennyadsl
Copy link
Member

What about a new extension called solidus_mobility as first iteration? It looks like the safer and cleaner path to me.

@fthobe
Copy link
Author

fthobe commented Apr 4, 2025

That's definetly the way to go

  • Solidus_mobility
  • Solidus_mobility_CMS

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