Change the semantics to have the classes that inherit from Noticed::Base called Notifiers #99
Conversation
…ase called Notifiers
|
I am going to revert the Gemfile.lock change, as that seems to be the issue with the Actions |
|
This will be a big one! I definitely think it will help with the wording. I think if you drop the Gemfile.lock and version changes, that will help doing a merge. 👍 |
…ced-classes-to-notifier # Conflicts: # README.md # test/dummy/noticed_test
|
@excid3: Yep, got ahead of myself. Reverted those changes, merged in the most recent pushes to |
|
Hi! Any idea when this will be merged? |
|
@excid3: Resolved conflicts with |
…on#to_notifier is called
…tize in the base DeliveryMethod#perform method
…fined for use in that method
# for use in that method
|
Oh man, I like this change a lot. It semantically/mentally decouples the AR model from the "handlers". I just started using this gem and based on the current semantics I thought the Felt like I should be able to do |
ghost
mentioned this pull request
|
Planning to get to this one soon! Been holding off on it because I knew it would be a major release and require users to update their code. |
|
All good. I figured. Let me know if/when you want me to update the PR with head of main |
|
If you want to, that would be great. I'm ready to go through with this. 👍 |
# Conflicts: # lib/noticed/delivery_methods/base.rb # lib/noticed/delivery_methods/email.rb # test/noticed_test.rb
|
@excid3: Updated against head of |
|
I rolled this into v2. 🎉 |
@excid3: I manually went thru every single file to try my best to not change any reference to the database
Notificationclass, but of course this should have a thorough set of second eyes. As you said in the issue, this is a breaking change so I bumped the version to 2.0.0. This also had the follow on effect of having the Rails version in theGemfile.lockbumped as well to 6.1.3resolves #98