search

mbleigh / acts-as-taggable-on

A tagging plugin for Rails applications that allows for custom tagging along dynamic contexts.
http://mbleigh.lighthouseapp.com/projects/10116-acts-as-taggable-on
MIT License
4.95k stars 1.18k forks source link

Fix `base_class` with Zeitwerk #1089

Closed f1sherman closed 1 year ago

f1sherman commented 1 year ago

Rails Zeitwerk autoloader is a lot more strict about when constants are loaded than the old autoloader. I noticed that in https://github.com/mbleigh/acts-as-taggable-on/pull/1079 base_class is being set to the actual class in an initializer, which results in an unintialized constant error when the autoloader is set to Zeitwerk. This fixes the setting to work in Zeitwerk by allowing it to be set to a String which gets constantized later, after initializers have run.

I also fixed the initializer example to call the setup method, which I believe is the recommended way to do that.

f1sherman commented 1 year ago

@donquxiote please let me know if I got anything wrong with this change, and thanks so much for your earlier contribution! I was super happy to see that multi-database support was already something that had been added.

donquxiote commented 1 year ago

@f1sherman this seems reasonable to me! I had initially written it this way but decided to be more direct and just use the class. I'll confirm this works in my use case this week but I don't foresee any issues.

donquxiote commented 1 year ago

@f1sherman sorry for the delay, but yeah using a string for the class would work in my case, and everything you changed looked good to me. I'm not a mod on this repo though, you'll probably want @seuros or one of the other owners to review it.

sundarraja commented 1 year ago

@seuros can this be merged?

seuros commented 1 year ago

Yes. I will work on this this week.

Let me know if there is other PRs that have to be merged.

sundarraja commented 1 year ago

Thanks much, @seuros :) I don’t have any other blocking issue.