Skip to content

Enable ActiveStorage specs against Rails 6.1#3886

Merged
kennyadsl merged 2 commits intosolidusio:masterfrom
nebulab:kennyadsl/activestorage-specs
Feb 15, 2021
Merged

Enable ActiveStorage specs against Rails 6.1#3886
kennyadsl merged 2 commits intosolidusio:masterfrom
nebulab:kennyadsl/activestorage-specs

Conversation

@kennyadsl
Copy link
Copy Markdown
Member

@kennyadsl kennyadsl commented Jan 8, 2021

Description

This PR is only useful to highlight the specs failures we have enabling ActiveStorage on Rails 6.1 and should be merged only after we fixed the existing issues.

Some ActiveStorage internals changed since we worked on this (see #2974 #3501) and we have to make some changes to reflect the new API.

@kennyadsl kennyadsl added this to the 2.11 milestone Jan 8, 2021
@kennyadsl kennyadsl self-assigned this Jan 8, 2021
@kennyadsl kennyadsl force-pushed the kennyadsl/activestorage-specs branch from ff0d6f4 to 480fc71 Compare February 3, 2021 16:55
@elia elia force-pushed the kennyadsl/activestorage-specs branch 3 times, most recently from 573d627 to 430e14b Compare February 12, 2021 16:55
@kennyadsl
Copy link
Copy Markdown
Member Author

@elia can you please elaborate a little bit about why we are using the paperclip partial for ActiveStorage as well?

@kennyadsl
Copy link
Copy Markdown
Member Author

@elia I did check the code and I think I've got it. We are now using the paperclip partial for both Paperclip and Activestorage. I think we can remove this abstraction in a separate PR as you probably thought.

@kennyadsl kennyadsl requested a review from a team February 15, 2021 15:56
Copy link
Copy Markdown
Member

@spaghetticode spaghetticode left a comment

Choose a reason for hiding this comment

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

@elia thanks!

@kennyadsl kennyadsl merged commit f6e84d9 into solidusio:master Feb 15, 2021
@kennyadsl kennyadsl deleted the kennyadsl/activestorage-specs branch February 15, 2021 22:31
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