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

Fix webpacker compatibility #1099

Merged
merged 1 commit into from Jan 30, 2024

Conversation

rioug
Copy link
Contributor

@rioug rioug commented Jan 24, 2024

As explain in #1098 this PR #1067 broke compatibility with Webpacker

Cause

The following line has been removed, resulting in an error when trying to access Webpacker::VERSION

require 'webpacker/version'

Fix

Re add the require 'webpacker/version' and for consistency also add require 'shakapacker/version' when using Shakapacker

Test

I tested it on our code base and our test suite is green.
Ideally someone using Shakapacker would give this ago as well

require 'webpacker/version' so that `Webpacker::VERSION` is accessible.
And for consitency, require 'shakapacker/version' when using Shakapacker
@rioug rioug force-pushed the 1098-fix-webpacker-compatibility branch from 0fb0f7a to 12f5115 Compare January 24, 2024 06:07
@rioug
Copy link
Contributor Author

rioug commented Jan 29, 2024

@unixmonkey Thanks for approving the PR !

I can see some the CI test failed but it doesn't seem to be related to the PR, should they be fixed ?
Also I can get the test to run on my local environment but I can see CI is passing with Ruby 3.0 and Rails 7, so I won't be spending more time looking at it.

@rioug rioug marked this pull request as ready for review January 29, 2024 03:21
@unixmonkey unixmonkey merged commit 549e00c into mileszs:master Jan 30, 2024
3 of 12 checks passed
@unixmonkey
Copy link
Collaborator

This is now released in version 2.8.0 of the Wicked PDF gem. Thank you so much for your help!

Please let me know if you have any issues!

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.

None yet

3 participants