Skip to content

Revert "Merge pull request #6122 from mamhoff/create-manifest-js-in-g…#6124

Merged
tvdeyen merged 1 commit intosolidusio:mainfrom
mamhoff:remove-manifest-generation
Feb 18, 2025
Merged

Revert "Merge pull request #6122 from mamhoff/create-manifest-js-in-g…#6124
tvdeyen merged 1 commit intosolidusio:mainfrom
mamhoff:remove-manifest-generation

Conversation

@mamhoff
Copy link
Copy Markdown
Contributor

@mamhoff mamhoff commented Feb 14, 2025

…enerated-apps"

This reverts commit bec42f6, reversing changes made to 37a5c8a.

Summary

This reverts #6122. If Rails 8 runs the install generator, the Rails process won't even start if the file is not present, and consequently our installer won't run either. The actual fix is this: solidusio/solidus_dev_support#229 - The sprockets manifest file needs to be present before even starting up a Rails process with Solidus in the Gemfile.

I guess we need to sort out the Sprockets removal next.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

…t-js-in-generated-apps"

This reverts commit bec42f6, reversing
changes made to 37a5c8a.
@mamhoff mamhoff requested a review from a team as a code owner February 14, 2025 13:44
@github-actions github-actions Bot added the changelog:solidus_core Changes to the solidus_core gem label Feb 14, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.68%. Comparing base (1b4e1f4) to head (e760610).
Report is 21 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6124   +/-   ##
=======================================
  Coverage   88.67%   88.68%           
=======================================
  Files         830      830           
  Lines       18016    18014    -2     
=======================================
  Hits        15976    15976           
+ Misses       2040     2038    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

I think we also need to fix it in bin/sandbox in this repo?

@tvdeyen
Copy link
Copy Markdown
Member

tvdeyen commented Feb 17, 2025

Or we wait for rails/sprockets-rails#546 to be merged and released?

@tvdeyen tvdeyen merged commit ce43cf9 into solidusio:main Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:solidus_core Changes to the solidus_core gem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants