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

Integrate non_stupid_digest_assets gem #2430

Merged
merged 3 commits into from
Feb 28, 2023
Merged

Conversation

afdev82
Copy link
Contributor

@afdev82 afdev82 commented Feb 22, 2023

What is this pull request for?

It integrates the patch from the non_stupid_digest_assets gem, since it is not compatible with Ruby 3.2 anymore.

Closes #2426

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

Sorry, something went wrong.

@afdev82 afdev82 force-pushed the iss2426 branch 4 times, most recently from 2bea7f8 to 6b6693b Compare February 22, 2023 15:34
Copy link
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.

Thanks.

Could you please make this 2 commits:

  1. Move the file into the lib folder
  2. Apply the fix for Ruby 3.2

This way the git history shows exactly what the fix for Ruby 3.2 was.

@afdev82
Copy link
Contributor Author

afdev82 commented Feb 27, 2023

Thanks.

Could you please make this 2 commits:

1. Move the file into the lib folder

2. Apply the fix for Ruby 3.2

This way the git history shows exactly what the fix for Ruby 3.2 was.

Thanks for the review, good idea, I will do that.

Use `File.exist?` instead of `File.exists?`
@afdev82
Copy link
Contributor Author

afdev82 commented Feb 28, 2023

Thanks.

Could you please make this 2 commits:

1. Move the file into the lib folder

2. Apply the fix for Ruby 3.2

This way the git history shows exactly what the fix for Ruby 3.2 was.

Done.

Copy link
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.

Thanks

@tvdeyen tvdeyen merged commit 4fc5380 into AlchemyCMS:main Feb 28, 2023
@tvdeyen
Copy link
Member

tvdeyen commented Mar 7, 2023

This does not work. Need to figure out why....

@afdev82 afdev82 deleted the iss2426 branch March 7, 2023 13:13
@afdev82
Copy link
Contributor Author

afdev82 commented Mar 7, 2023

This does not work. Need to figure out why....

Hi Thomas, what exactly does not work? I have applied the patch in production in two websites and the tinymce editor is showing correctly and the assets are precompiled fine.

@tvdeyen
Copy link
Member

tvdeyen commented Mar 7, 2023

The whitelist is using Regexp and since the code does not use Regexp to compare the files it needs to create assets without digests for it skips all the tinymce files. Fix is here https://github.com/AlchemyCMS/alchemy_cms/pull/2436/files#diff-c77ddfb34e3a1dae0510164a25a6251cf8532bd8a8cff567bda403b8010cc04e

@tvdeyen
Copy link
Member

tvdeyen commented Mar 7, 2023

Open a page that uses Tinymce. It will not load because it cannot find our alchemy_link plugin

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.

Compatibility with Ruby 3.2
2 participants