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

Remove method missing helper #1623

Merged
merged 1 commit into from
Nov 30, 2016

Conversation

gevann
Copy link

@gevann gevann commented Nov 28, 2016

This PR removes the method_missing override from core base helpers, which removes the custom image helper functionality and accompanying specs.

style if style.in? possible_styles.with_indifferent_access
end
end

def create_product_image_tag(image, product, options, style)
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove this also

Copy link
Contributor

@jhawthorn jhawthorn left a comment

Choose a reason for hiding this comment

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

👍 other than one comment.

Could you also squash this into a single commit. The tests are failing until the final spec removal.

@gevann gevann force-pushed the remove-method-missing-helper branch from 72d447b to c372425 Compare November 29, 2016 02:06
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.

Please!

@gevann gevann force-pushed the remove-method-missing-helper branch from c372425 to dcb72f2 Compare November 30, 2016 00:42
Remove image_style_from_method_name

Remove define_image_method

Remove custom-image-helper-context specs.

Custom image helpers have been removed. Since this feature is no
longer provided, remove test of them.

Remove create_product_image_tag
@gevann gevann force-pushed the remove-method-missing-helper branch from dcb72f2 to a5f9c02 Compare November 30, 2016 00:48
@jhawthorn jhawthorn merged commit e460788 into solidusio:master Nov 30, 2016
@tvdeyen
Copy link
Member

tvdeyen commented Nov 30, 2016

The day has finally come 👏🏻

@mtomov
Copy link
Contributor

mtomov commented Dec 1, 2016

Hah, indeed so Thomas .. those have been quite a trickery to grep. Thanks a lot!

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.

4 participants