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.

Sign up for GitHub

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

Jump to bottom

Remove unused mini_racer gem #10915

Merged
merged 1 commit into from
Jun 12, 2023
Merged

Remove unused mini_racer gem #10915

merged 1 commit into from
Jun 12, 2023

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented May 31, 2023

What? Why?

All asset compilation is done with webpacker at the moment.

What should we test?

  • Go through checkout as sanity check.

Release notes

Changelog Category: Technical changes

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

@mkllnk mkllnk self-assigned this May 31, 2023
@mkllnk mkllnk marked this pull request as ready for review May 31, 2023 23:38
@mkllnk mkllnk requested a review from Matt-Yorkley May 31, 2023 23:38
dacook
dacook approved these changes Jun 1, 2023
View reviewed changes
Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

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

If tests pass, it sounds good to me!

jibees
jibees approved these changes Jun 1, 2023
View reviewed changes
Copy link
Contributor

@jibees jibees left a comment

Choose a reason for hiding this comment

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

👌

@Matt-Yorkley
Copy link
Contributor

Matt-Yorkley commented Jun 1, 2023 •
edited
Loading

All asset compilation is done with webpacker at the moment.

That's not strictly true, we still have a bunch of legacy JS and Angular templates under app/assets/javascript/* (which don't play nicely with modern JS compiling) and they're not currently compiled by webpacker. I have a feeling they might still need the v8 bridge provided by mini_racer, but I could be wrong. 🤷‍♂️

It might be worth testing this a bit before merging, maybe deploy to staging and check a few bits that use angular templates and maybe try it in the Docker setup if anyone is using that in dev?

@mkllnk
Copy link
Member Author

mkllnk commented Jun 1, 2023

I just tried in normal dev environment changing an AngularJS template and reloading the page. It all updated and I had mini_racer uninstalled completely.

@mkllnk @drummer83
Remove unused mini_racer gem
d6859e0 
All asset compilation is done with webpacker at the moment.
@drummer83 drummer83 self-assigned this Jun 12, 2023
@drummer83 drummer83 added pr-staged-uk staging.openfoodnetwork.org.uk and removed pr-staged-uk staging.openfoodnetwork.org.uk labels Jun 12, 2023
@drummer83 drummer83 merged commit 80dd875 into openfoodfoundation:master Jun 12, 2023
@drummer83
Copy link
Contributor

Hi @mkllnk,
Thanks for the test instructions. I checked out successfully using Stripe 3DS2 authentication. Looking good.
Merging! 🚀

@drummer83 drummer83 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Jun 12, 2023
@mkllnk mkllnk deleted the mini-racer branch July 28, 2023 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jibees jibees jibees approved these changes

@dacook dacook dacook approved these changes

@Matt-Yorkley Matt-Yorkley Awaiting requested review from Matt-Yorkley

Assignees

@drummer83 drummer83

@mkllnk mkllnk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mkllnk @Matt-Yorkley @drummer83 @jibees @dacook