Support configuring from return type #73
Conversation
|
Ping @jayroh and @theycallmeswift |
|
Interesting. Code looks fine, needs to be documented though. What's your use case for this? |
|
Yeah what Swift said. @calebthompson can you give some more context in the PR description? |
|
Documented. |
|
The use case was @r38y's. He mentioned it here: #69 (comment) |
spec/griddler/email_spec.rb
Outdated
There was a problem hiding this comment.
I'm weary of the context of these two tests. The description states that we're testing the body formatting but these are with regards to the to and from email addresses. For now I'd suggest we consider pulling these out into their own describe.
In the future I think a good bit of the responsibility in our Email class should get extracted to more focused and relevant classes. What do you guys think?
There was a problem hiding this comment.
I think a concerted effort to prevent bloat in the Email class is a good idea. I think the things that are in there now are fairly relevant though. Do you disagree?
There was a problem hiding this comment.
I was thinking a few days ago that it would probably be a good idea to have an email builder and turn Griddler::Email into a value object.
|
I just have that one comment, Caleb. Other than that it looks really good. And it's a good call. |
* Add default from configuration of :email * Extract shared examples for email address config and apply them to both #to and #from * Document `from` options
|
I could have sworn I submitted this as a PR. I guess I was hallucinating or did the work and forgot to. Anywho, the thing I wanted in the end, configuring from return type, is now in! Hurrah! |
|
Aha! Here is my branch: https://github.com/r38y/griddler/tree/from-hash I may have to submit some of those commits because some changes needed to happen to the Postmark adapter. Ignore me, carry on. |
No description provided.