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

Revert to the ruby racer #3244

Merged
merged 1 commit into from
Jan 10, 2019
Merged

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented Dec 26, 2018

What? Why?

I can't make mini racer work locally (osx mojave) . I cant trace back to the point of failure but it did work at some point.
This is not only my problem as 2 new contributors (I believe all using osx as I am) have also reported this problem.

So, this PR reverts PR #2424.
I suggest we re-open #2100

This PR makes my life (and some new contributors with the same problem) a lot easier but makes the assets compilation move back from 211s to 316s (as noted on #2100).

What should we test?

Asset precompilation should still work correctly. We should make a sanity check of the app.

Release notes

Moved back to therubyracer for asset compilation to avoid trouble caused by the faster mini racer.
Changelog Category: Changed

…me developers using osx.

We will probably be able to move back to the faster mini racer when we upgrade to newer rails versions
@luisramos0
Copy link
Contributor Author

this build has broken twice already with random errors...
I wonder if miniracer made the build more stable, but I dont udnerstand how that could be...

@luisramos0
Copy link
Contributor Author

I have staged this in ES and confirmed precompile assets didn't error out.

@sauloperez
Copy link
Contributor

We should keep an eye on Mojave's support for libv8 and revert this change in the future if we can. I guess that's the underlying problem.

@luisramos0
Copy link
Contributor Author

staged this in ES to test asset compilation, I am keeping it there because this can also be used to try to replicate potential problem with variant overrides.

@Matt-Yorkley
Copy link
Contributor

Wow, 211s to 316s is a big jump. Is there an issue open still to investigate speeding up the precompile stage? Should we look for alternatives?

@sauloperez
Copy link
Contributor

IMO it's better if we focus on getting our app and dependencies up to date. This is just a symptom of how bad our situation is. Chances are that any other solutions might not work with deprecated Ruby and Rails versions...

Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Yes, we need to update our dependencies to find better solutions.

@sigmundpetersen
Copy link
Contributor

Is this still on ES @luisramos0 @sauloperez ? Is it tested and OK?

@luisramos0
Copy link
Contributor Author

this is on ES yes.
it needs a sanity check from a tester.

@RachL RachL self-assigned this Jan 9, 2019
@RachL
Copy link
Contributor

RachL commented Jan 9, 2019

@luisramos0 @sauloperez staging ES URL redirects now to https://appstaging.katuma.org/ which I cannot access apparently... am I looking at the wrong place?

@sauloperez
Copy link
Contributor

sauloperez commented Jan 9, 2019

Nope, that's me. I didn't properly restore staging when I played with it last week. I'll fix it now.

EDIT @RachL staging.katuma.org is back online. You will surely need to disable cache either checking "Disable cache" from the Network tab of the dev tools or from a incognito window.

@RachL
Copy link
Contributor

RachL commented Jan 10, 2019

thx @sauloperez I'm starting testing now :)

@RachL
Copy link
Contributor

RachL commented Jan 10, 2019

@mkllnk mkllnk merged commit 935e233 into openfoodfoundation:master Jan 10, 2019
@luisramos0 luisramos0 deleted the ruby-racer branch January 11, 2019 11:20
@luisramos0
Copy link
Contributor Author

oh my god, I cant believe, my master just works now :-D

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.

6 participants