Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mandrill Merge Vars #219

Merged
merged 4 commits into from
Oct 7, 2016

Conversation

jbernardo95
Copy link
Contributor

@jbernardo95 jbernardo95 commented Oct 1, 2016

This PR resolves #39

Adds a function to easily add Mandrill Merge Vars to an email.

Example:

email |> put_merge_vars(users, fn(user) -> %{first_name: user.first_name} end)

@jbernardo95
Copy link
Contributor Author

@paulcsmith Can I get a review here ? 🙂

}
])

## Example
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about moving the example above the "A convenience function for:"

]

email = new_email
|> MandrillHelper.put_merge_vars(users, fn(user) -> %{ full_name: user.full_name } end)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you indent this two spaces? Either that or put it on the same line and make the function multiline like this:

email = MandrillHelper.put_merge_vars email, users, fn(user) ->
  %{full_name: user.full_name}
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still trying to get used to the code standards x)

Fixed.

@paulcsmith
Copy link
Contributor

@jbernardo95 Thanks for the PR. This looks awesome! Just a couple comments

@jbernardo95
Copy link
Contributor Author

@paulcsmith Updated.

@paulcsmith paulcsmith merged commit 67d6ac8 into beam-community:master Oct 7, 2016
@paulcsmith
Copy link
Contributor

Thanks @jbernardo95! Nice work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a function to easily add merge vars using the MandrillAdapter
2 participants