Add type specs for address functions #164
Conversation
| @@ -68,11 +68,12 @@ defmodule Bamboo.Email do | |||
| """ | |||
|
|
|||
| @type address :: String.t | {String.t, String.t} | |||
| @type address_list :: nil | address | [address] | |||
There was a problem hiding this comment.
or any because it can be anything that implements Bamboo.Formatter protocol
There was a problem hiding this comment.
Can you explain that more?
I don't like any if it can be avoided. That doesn't allow type checking anymore.
There was a problem hiding this comment.
You can implement the Bamboo.Formatter protocol for anything. Maps, structs, atoms, boo leans, anything that protocols allow you to implement. So any value you want could be formatted. We could say that it accepts a map or list of maps, but that wouldn't really be technically correct :S
I suppose we could just say that anything other than a map/struct would be weird to format with a protocol and just add a type for that but I'm not totally sure about that.
Or maybe there a way to add a type for anything that implements a protocol? That would work best if possible.
Sent from my iPhone
On May 24, 2016, at 8:52 AM, Jason Draper notifications@github.com wrote:
In lib/bamboo/email.ex:
@@ -68,11 +68,12 @@ defmodule Bamboo.Email do
"""@type address :: String.t | {String.t, String.t}
- @type address_list :: nil | address | [address]
Can you explain that more?I don't like any if it can be avoided. That doesn't allow type checking anymore.
—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
|
This is a great idea! Just left a couple comments :) |
|
@paulcsmith updated! |
|
|
||
| ``` | ||
| new_email | ||
| |> #{function_name}(["sally@example.com", "james@example.com"]) |
There was a problem hiding this comment.
We've been indenting by 4 spaces instead of using ```. I think after that's changes this can be merged in :D
There was a problem hiding this comment.
Also, what do you think of linking to the docs in Bamboo.Formatter? Something like
You can pass in a string, list of string, or anything that implements the `Bamboo.Formatter` protocol. See `Bamboo.Formatter` for more examples.
That might help people figure out what they can use for recipients. What do you think?
There was a problem hiding this comment.
We've been indenting by 4 spaces instead of using ```. I think after that's changes this can be merged in :D
Uh...you sure about that? https://github.com/thoughtbot/bamboo/blob/master/lib/bamboo/email.ex#L21-L34
https://github.com/thoughtbot/bamboo/blob/master/lib/bamboo/email.ex#L42-L67
There was a problem hiding this comment.
Almost everywhere else we are
https://github.com/thoughtbot/bamboo/blob/master/lib/bamboo/adapter.ex#L15
https://github.com/thoughtbot/bamboo/blob/master/lib/bamboo/formatter.ex#L39
https://github.com/thoughtbot/bamboo/blob/master/lib/bamboo/phoenix.ex#L12
https://github.com/thoughtbot/bamboo/blob/master/lib/bamboo/test.ex#L31
https://github.com/thoughtbot/bamboo/blob/master/lib/bamboo/plug/email_preview_plug.ex#L13
https://github.com/thoughtbot/bamboo/blob/master/lib/bamboo/adapters/mailgun_adapter.ex#L10
Thanks for pointing out Bamboo.Email though. Those should be changed to 4 spaces of indentation. Oops!
There was a problem hiding this comment.
Added a PR to change those examples: #165
Adding these makes it easier for users to see what options can be passed into each of these functions.
|
LGTM! This will make it a lot easier for people to figure out what can be done with recipients. Thank you very much! |
I recently ran into an issue where I was trying to figure out why a comma separated string wasn't working for the
toand I needed to see what type was supported when calling thetofunction. I think by adding these it makes it easier for users to see what options are passed but also it means that users could perform static analysis to make sure they're calling it correctly.