Skip to content

remove annotate warnings and comments #873

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

Merged
merged 2 commits into from
Aug 10, 2025
Merged

Conversation

adrienpoly
Copy link
Collaborator

following to drwl/annotaterb#172 this PR removes teh magic comment required in associated objects to silence the annotations warnings

Normally the problem should be fix with the new annotaterb gem and active_record-associated-object but at the time of this writing when I run the command I still have those warnings

❯ bundle exec annotaterb models
Annotating models
Unable to process app/models/event/cfp.rb: undefined method 'cfp' for true
Unable to process app/models/event/schedule.rb: undefined method 'schedule' for true
Unable to process app/models/event/static_metadata.rb: undefined method 'static_metadata' for true
Unable to process app/models/organisation/static_metadata.rb: undefined method 'static_metadata' for true
Unable to process app/models/speaker/profiles.rb: undefined method 'profiles' for true
Unable to process app/models/talk/agents.rb: undefined method 'agents' for true
Unable to process app/models/talk/downloader.rb: undefined method 'downloader' for true
Unable to process app/models/talk/thumbnails.rb: undefined method 'thumbnails' for true
Model files unchanged.

@adrienpoly adrienpoly marked this pull request as draft August 5, 2025 06:09
@marcoroth marcoroth force-pushed the main branch 2 times, most recently from d35f39a to ec30df0 Compare August 9, 2025 18:51
kaspth added a commit to kaspth/active_record-associated_object that referenced this pull request Aug 10, 2025
Our `method_missing` based proxying is technically eager to hopefully allow us to support more of Active Record's surface area cheaply. It's been working well for over 3 years, but there's some issues.

Notably, a `record.respond_to?` check is not enough and there's certain methods we don't want to proxy.

Stuff like Active Record's class-level configuration API via `abtract_class?`, `descends_from_active_record?`, `abstract_class=` and `primary_abstract_class=` to name a few.

So ideally opting out of proxying methods that end in `?` and `=` should be safe.

I'm not including `!` to allow `find_by!` to work.

Ref: rubyevents/rubyevents#873
kaspth added a commit to kaspth/active_record-associated_object that referenced this pull request Aug 10, 2025
#42)

Our `method_missing` based proxying is technically eager to hopefully allow us to support more of Active Record's surface area cheaply. It's been working well for over 3 years, but there's some issues.

Notably, a `record.respond_to?` check is not enough and there's certain methods we don't want to proxy.

Stuff like Active Record's class-level configuration API via `abtract_class?`, `descends_from_active_record?`, `abstract_class=` and `primary_abstract_class=` to name a few.

So ideally opting out of proxying methods that end in `?` and `=` should be safe.

I'm not including `!` to allow `find_by!` to work.

Ref: rubyevents/rubyevents#873
Ref: #40
@kaspth
Copy link
Contributor

kaspth commented Aug 10, 2025

I just shipped a new release that should fix this https://github.com/kaspth/active_record-associated_object/releases/tag/v0.9.3

I verified the fix on this PR and it shouldn't bail on Associated Objects anymore. There's a separate failure about an enum column not being backed by a database column now though.

@adrienpoly
Copy link
Collaborator Author

Thanks @kaspth

for the enum error I know where this is coming from when you have an old db that didn’t run the migrations from a long time

I have a fix that I will push in another PR

@adrienpoly adrienpoly force-pushed the remove-annotate-rb-warnings branch from f99c117 to 7449ea1 Compare August 10, 2025 20:42
@adrienpoly adrienpoly marked this pull request as ready for review August 10, 2025 20:47
@adrienpoly adrienpoly merged commit 8601dc6 into main Aug 10, 2025
4 checks passed
@adrienpoly adrienpoly deleted the remove-annotate-rb-warnings branch August 10, 2025 20:47
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.

2 participants