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 middleman v4 compatibility issues #9

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

schmidt
Copy link

@schmidt schmidt commented Sep 25, 2017

I was facing a number of incompatibilities, when trying to use middleman-smusher with a current middleman v4.2.1. This pull request tries to address all of them. Please take a look at the individual commits for a detailed description of the changes.

Since the separate file was defining a Middleman::Smusher module, while in fact, Middleman::Smusher is a subclass of Middleman::Extension. This caused conflicts, when loading the extension.

Removing the separate file and adding the version number directly to the gemspec was the simplest solution. Changing the declaration within version.rb to the proper class definition would defeat the idea of a simple, easy-to-load version file, which can be used from the gemspec without further prerequisites.
Since the non-ssl version is discouraged and insecure.
We only need to load smusher, when the extension is activated.
* loading middleman-core as recommended by the default plugin skeleton
* fix null vs. nil default value declaration
* `build_dir` is no longer available, using `app.config[:build_dir]` instead
* `say_status`: builder does not return thor instance directly anymore
* Current Smusher API provides `convert_gifs` option, not `with_gifs`.

Also updating middleman version requirement, to make sure that the code is not used with older versions of middleman.
@Ffoeg311
Copy link

Is there a reason this hasn't been merged? Running into some smusher compatibility issues on our recent bundle update and I think this may be useful.

@schinns
Copy link

schinns commented Apr 22, 2019

I am also running into similar issues. Looking forward to this PR being merged.

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.

3 participants