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

Revert class spoofing #933

Merged

Conversation

Alexander-Senko
Copy link
Collaborator

Decorators shouldn't pretend to be instances of the underlying model classes for the following reasons:

  1. It's terribly wrong to pretend being a class without implementing it's interface. It can break third party code.
  2. Hacking Ruby core methods can lead to unexpected and hard to understand behavior. It should be avoided without a strong reason behind.
  3. I'd prefer compatibility patches to be narrow-scoped and live in there own modules.

References

Decorators shouldn't pretend to be instances of the underlying model 
classes for the following reasons:

1. It's terribly wrong to pretend being a class without implementing 
it's interface. It can break third party code.
2. Hacking Ruby core methods can lead to unexpected and hard to 
understand behavior. It should be avoided without a strong reason 
behind.
3. I'd prefer compatibility patches to be narrow-scoped and live in 
there own modules.

Resolves drapergem#859.
Reverts  drapergem#72,
         drapergem#110,
         drapergem#417,
         drapergem#497.
Copy link
Collaborator

@y-yagi y-yagi left a comment

Choose a reason for hiding this comment

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

I agree that there is no reason to maintain this behavior. This would be a breaking change, but I think it's enough to write a document(like uprading guide) when we needed.

LGTM!

@Alexander-Senko Alexander-Senko merged commit 93a9bc2 into drapergem:master Sep 13, 2024
8 checks passed
@Alexander-Senko Alexander-Senko deleted the revert/class-spoofing branch September 13, 2024 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants