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

Re-added assetable on attachments #778

Closed
wants to merge 1 commit into from

Conversation

johannlejeune
Copy link

Basically a revert of 4992843 since I didn't quite understand why it was removed in the first place and I think it can still be useful to a lot of people.

@dbourguignon
Copy link

I don't understand the remove of assessable either.
It's a breaking change

I will try your fork for now

@galetahub any backstory for this? I'd like to understand

@botularius
Copy link

I also don't understand the remove of assetable. As I did not see this pull request before I opended an issue here:
#788

@stale
Copy link

stale bot commented Feb 2, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Feb 2, 2019
@stale stale bot closed this Feb 9, 2019
@januszm
Copy link

januszm commented May 7, 2023

Per @galetahub comment: #683 (comment)

It looks like idea was to adapt CKeditor to the change in Rails where belongs_to is required by default, but the chosen solution is not ideal becase the feature, instead of being actually adapted to be 'optional' was simply removed.

@januszm
Copy link

januszm commented May 7, 2023

But just to be fair, perhaps this feature was used so rarely and can potentially be implemented outside the scope of this gem that it was better to remove it. In all my projects, I realized, I haven't used it so it's not really a breaking change for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants