Extract adapters to standalone gems

[calebhearth]

Adapters have proven to be:

1. Embarrassing, as they seem to often be broken, and
2. Difficult to maintain as neither I nor others at thoughtbot use the other 'supported' services.
   • [x] Extract gems for each service, including SendGrid
   • [x] Extract Sendgrid https://github.com/thoughtbot/griddler/issues/130
   • [x] Extract Mandrill https://github.com/thoughtbot/griddler/issues/128
   • [x] Extract CloudMailin https://github.com/thoughtbot/griddler/issues/126
   • [x] Extract Postmark https://github.com/thoughtbot/griddler/issues/129
   • [x] Extract Mailgun https://github.com/thoughtbot/griddler/issues/127

I'm willing to continue working on griddler-sendgrid and on griddler proper, but after extraction I don't plan to continue actively maintaining the adapters and will request that those using them or representatives of the companies providing the service take over.

We'll add links to the various adapters in the README and make sure that people understand the division of responsibility as far as issues / pull requests.

[r38y]

I'm mostly interested in the Postmark one and am willing to extract and maintain it. Would griddler-sendgrid be the one we would look to as an example?

I've touched the other adapters but don't know a ton about them.

[calebhearth]

My vision for how these will be extracted is:

1. into a gem named griddler-<service>, such as griddler-postmark.
2. The gem includes the adapter code in something like lib/griddler/<service>/adapter and is required as griddler/service. This means a rename of the adapter class name to, for example, Griddler::Sendgrid::Adapter.
3. I'll work to set up a file that can be required to test against griddler itself, similar to https://github.com/thoughtbot/griddler/blob/master/spec/features/adapters_and_email_spec.rb. I'll update everyone on that. My first thought is a shared_example that you can run in your gem.
4. We'll have to change the way we are finding adapters since we'll want to be able to include any adapter that is created without needing to add it to a list in Griddler. The two options I can think of are to register your adapter or to constantize based on the configuration name (email_service = :postmark => Griddler.const_get(email_service.classify) I think I like the latter option but I don't feel strongly.

So the code you'll need from Griddler to make the new gems will be the adapter and its test.

[calebhearth]

The adapters would have griddler as a dependency, which will also allow you to lock down versions to protect against API changes if you need to. Just remember that doing so will mean that new features won't be available to your users until you update the version lock.

[wingrunr21]

Griddler itself will need either additional defensive code around data coming from the adapters or thorough documentation on its expectations for how the adapters should normalize the data. Without these we'll deal with more issues similar to what we just fixed with Mandrill (for background, Mandrill was handing Griddler nil when Griddler didn't expect nil to ever come through).

[calebhearth]

Yeah that's a good point. We'll want a section in the README that goes over that users will need to add adapters to their gems, and another to talk about creating an adapter and our expectations. I'd rather have that documented than code against the adapters - we can push people to the adapters' github projects when data is coming through wrong.

[wingrunr21]

Agreed. It would also be nice to maintain some kind of API test that can be required in the adapter's test suite but would still allow Griddler to dictate the API itself.

[calebhearth]

That means we'll probably also want:

• [ ] A custom error for not having an adapter specified
• [ ] A custom error for when Griddler::Email is given unacceptable parameters (which means at least some defensive code)

[calebhearth]

Agreed. It would also be nice to maintain some kind of API test that can be required in the adapter's test suite but would still allow Griddler to dictate the API itself.

Yeah that's what I was thinking # 3 above would become.

[calebhearth]

Here's the start of the adapter support on Griddler's side: https://github.com/thoughtbot/griddler/tree/adapter-gem-support

[scsmith]

In terms of 4, I'm also not sure that I've got any really strong feelings towards how we find adapters, but it would be nice to consider the ability for CloudMailin to register multiple adapters. We have a few different formats (original, multipart, json, raw) admittedly, we're better only supporting the core one (multipart) but we have had customers ask if they can use others with griddler.

[calebhearth]

Since I think we're moving away from hard coding them in Griddler, any adapter should be able to either register itself or be looked up.

# register in your gem
Griddler.register_adapter(:cloudmailin_json)
# or look up when the user specifies
Griddler.const_get(:cloudmailin_json.to_s.classify)

[calebhearth]

I'm currently working on extracting the SendGrid adapter, by the way, so hopefully that will be working soon as a reference.

[calebhearth]

Okay @gabebw was kind enough to finish setting up the infrastructure for plugging adapters in, as well as doing the last little bit on griddler-sendgrid registering itself to Griddler.

I think a logical next step is to get griddler-demo running on the master branches of both griddler and griddler-sendgrid to make sure things work as expected, then we can get the other adapters moved out.

Sorry, this sort of fell off my plate with a move and a bunch of travel, but hopefully we can get back on track now.

[wingrunr21]

:thumbsup: sounds like a plan!

[gabebw]

@calebthompson I edited your top-level comment to have sub-lists. I assume we're going to maintain griddler-sendgrid to I don't have a "Find a maintainer for Sendgrid" checkbox.

[calebhearth]

Okay griddler-demo is good to go #130 is closed and we're ready to have other implementations extracted.

@r38y @wingrunr21 @scsmith @bradpauly

[wingrunr21]

Awesome. I'll take a look tonight.

[calebhearth]

@gabebw There are actually other issues for each adapter, and they all have people who have offered to maintain them. As soon as the adapters are all good to go, we're also passing off griddler-sendgrid to @sendgrid.

[gabebw]

Oh cool. I'll stop meddling and let you edit your comment as you see fit then.

[calebhearth]

All set.

[wingrunr21]

Is griddler going to have a hard dependency on griddler-sendgrid? Right now I need to add griddler-sendgrid to the Gemfile in order to work on the adapter due to this line.

There's also a potentially stray SendGrid reference here

What's happening to the be_normalized_to helper? Is that getting moved into the testing module?

[gabebw]

@wingrunr21 No, @calebthompson pointed out in a PR (I can't find it now) that that require "griddler/sendgrid" line shouldn't be in griddler proper, but should be part of your application.

[wingrunr21]

Ok, cool.

[calebhearth]

We want it in for testing, at least for now. Same with other adapters. Feel free to add yours to Gemfile like griddler-sendgrid.

On Tue, Jun 3, 2014 at 4:53 PM, Gabe Berke-Williams notifications@github.com wrote:

@wingrunr21 No, @calebthompson pointed out in a PR (I can't find it now) that that require "griddler/sendgrid" line shouldn't be in griddler proper, but should be part of your application.

Reply to this email directly or view it on GitHub: https://github.com/thoughtbot/griddler/issues/124#issuecomment-45025662

[calebhearth]

@wingrunr21 @r38y @scsmith @bradpauly @scottmotte 1.0 has been released.

Please make sure you've cut gems for your adapters so that the upgrade path is clear.

[bradpauly]

Done. https://github.com/bradpauly/griddler-mailgun/releases/tag/v1.0.0

[wingrunr21]

Done

[r38y]

BOOOM. Done. https://rubygems.org/gems/griddler-postmark