-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Skip ActionCable
for dummy apps
#5420
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. The installer spec is failing. Mind to take a look?
Rails 7.1 came out. |
True dat. Not sure why the installer spec fails if this happens 🤔 |
I think it's because the installer specs simulate the install process, which uses Rails at the latest available version. We need to investigate, cc @elia @rainerdema @stefano-sarioli |
That's correct, I'm working on fixing the CircleCI script, meanwhile I think we can merge this, knowing the failure is a false negative 👍 |
#5421 has been merged, rebasing should fix the CI |
We don't really use ActionCable for anything as far as I know. Having it makes installing Turbo/Stimulus in extensions harder. The installer for Turbo will look for a Gemfile in `Rails.root`, which is not there in case of the dummy app, and fail. If, however, `config/cable.yml` can't be found, the Turbo installer succeeds with a message that Turbo is installed, just without the broadcasting feature (which is alright for most apps I believe).
e83bb5a
to
3f8ad82
Compare
Codecov Report
@@ Coverage Diff @@
## main #5420 +/- ##
==========================================
- Coverage 88.86% 88.85% -0.01%
==========================================
Files 607 607
Lines 14747 14748 +1
==========================================
Hits 13105 13105
- Misses 1642 1643 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Yes, this does need to be backported to solidus 3.4 - 4.2. I can't add the backport labels due to lack of permission. Can anyone? |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
Summary
We don't really use ActionCable for anything as far as I know. Having it makes installing Turbo/Stimulus in extensions harder.
The installer for Turbo will look for a Gemfile in
Rails.root
, which is not there in case of the dummy app, and fail. If, however,config/cable.yml
can't be found, the Turbo installer succeeds with a message that Turbo is installed, just without the broadcasting feature (which is alright for most apps I believe).Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: