thoughtbot / griddler

Simplify receiving email in Rails (deprecated)
http://griddler.io/
MIT License
1.38k stars 199 forks source link

Adapter specific configuration #225

Closed wingrunr21 closed 4 months ago

wingrunr21 commented 9 years ago

Wanted to open an issue on this before putting together a PR.

I've got a use case where a user would like a configuration option for the Mandrill adapter (see wingrunr21/griddler-mandrill#23). Right now I am considering three approaches:

Any thoughts on this?

gabebw commented 9 years ago

If I understand your first bullet point correctly, then this line changes from

processor_class.new(email).public_send(processor_method)

to

if extra_configuration?
  processor_class.new(email, extra_configuration).public_send(processor_method)
else
  processor_class.new(email).public_send(processor_method)
end

(I added the if so that adapters that only take 1 arg don't break.) I like that version the best.

wingrunr21 commented 9 years ago

Ya, that's what I meant.

We can also check the arity of new to protect adapters or increment the griddler version (since this is an API change) and just have adapters depend on the older version until they update.

gabebw commented 9 years ago

increment the griddler version (since this is an API change)

Yes, good point. I like that idea better, since as you point out, this is a breaking change.

micahbf commented 6 years ago

Would love to see this merged! I ran into the exact issue @wingrunr21 describes - we need to be able to configure SPF checking in griddler-mandrill. In the mean time we will probably need to maintain our own fork of the adapter gem.

wingrunr21 commented 6 years ago

@micahbf agreed, I need to get back on getting this change pushed out. I'm a bit swamped right now but I will try and find time.

micahbf commented 6 years ago

@wingrunr21 Let me know if I can help.