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

Modularize global required method #168

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

czeise
Copy link

@czeise czeise commented Sep 7, 2023

This PR fixes #45 and builds off of mful's previous PR for this issue.

It should be noted that #45 specifically mentions a compatibility issue with FactoryBot, but the issue is much broader than that. This gem currently introduces a required method globally in the projects that use it, which breaks any method named required in those projects.

This solution:

  • Moves the required method to a new Asana::CompatibilityHelper module that can be used safely throughout this codebase without making it a global method in any project it's used in.
  • Does not modify generated code--to the best of my knowledge, let me know if I've missed something!

My concerns with this solution:

I believe the method that I'm protecting here (required) is only necessary to add support to a ruby version (2.0.0) that isn't actually supported by this gem. It likely could be removed, but it would require updates to the code generation codebase.

Supported rubies:

  • MRI 2.7.x
  • MRI 3.0.x
  • MRI 3.1.x

* Namespace global required method

* Update required call usage

* Add additional required usage tests

* Remove debugging code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

ruby2_0_0_compatibility.rb#required Breaks FactoryGirl on models that have a required attribute.
1 participant