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

Update README.md #552

Closed
wants to merge 1 commit into from
Closed

Conversation

Raulkumar
Copy link

No description provided.

Copy link
Collaborator

@germsvel germsvel left a comment

Choose a reason for hiding this comment

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

Thanks for opening these changes @Raulkumar. There are a few places where the original language was correct. Do you think you could revert those changes?

- **Deliver emails in the background**. Most of the time you don't want or need
to wait for the email to send. Bamboo makes it easy with
Mailgun, and SendGrid][available-adapters]. It's also quite easy to write your delivery adapter if your platform isn't yet supported.
- **Deliver emails in the background**. Most of the time you don't want or need to wait for the email to send. Bamboo makes it easy with
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think the text change here, correct? Could we leave this as it was? I think we prefer the text wrapping for legibility.

`Mailer.deliver_later`.
- **A functional approach to mail delivery**. Emails are created, manipulated,
and sent using plain functions. This makes composition a breeze and fits
naturally into your existing Elixir app.
and sent using plain functions. This makes composition a breeze and fits naturally into your existing Elixir app.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we undo this change? I think we prefer the text wrapping.

- **Unit test with ease**. Bamboo separates email creation and email delivery
allowing you to test by asserting against email fields without the need for
special functions.
- **Dead-simple integration tests**. Bamboo provides helper functions to make
- **dead-simple integration tests**. Bamboo provides helper functions to make
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this made more sense as it was, but I'm happy to change this if it makes more sense. Why did you think we should lower case the d?

@@ -150,7 +147,7 @@ defmodule MyApp.SomeControllerPerhaps do
end
```

Your application is now set up to send email with Bamboo! :tada:
Your application is now set up to send an email with Bamboo! :tada:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this used to be correct before. The whole application was set up to send email (not just an email) with Bamboo. Does that make sense?

@@ -190,29 +187,29 @@ config :my_app, MyApp.Mailer,
```

You can create new adapters for any environment by implementing the
[`Bamboo.Adapter`] behaviour.
[`Bamboo.Adapter`] behavior.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The previous spelling was the correct one. Behaviours (with the u) are part for Elixir and Erlang. So we want to keep the other spelling to communicate that we mean an Elixir/Erlang behaviour.

process completion (e.g. a web request in Phoenix). Bamboo provides a
`deliver_later` function on your mailers to send emails in the background. It
also provides a [`Bamboo.DeliverLaterStrategy`] behaviour that you can
also provides a [`Bamboo.DeliverLaterStrategy`] behavior that you can
Copy link
Collaborator

Choose a reason for hiding this comment

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

The previous spelling was the correct one. Behaviours (with the u) are part for Elixir and Erlang. So we want to keep the other spelling to communicate that we mean an Elixir/Erlang behaviour.

linking to the calling process, so errors in the mailer won't bring down your
app.

You can also create custom strategies by implementing the
[`Bamboo.DeliverLaterStrategy`] behaviour. For example, you could create
[`Bamboo.DeliverLaterStrategy`] behavior. For example, you could create
Copy link
Collaborator

Choose a reason for hiding this comment

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

The previous spelling was the correct one. Behaviours (with the u) are part for Elixir and Erlang. So we want to keep the other spelling to communicate that we mean an Elixir/Erlang behaviour.

germsvel added a commit that referenced this pull request Feb 18, 2021
Supersedes: #552

What changed?
==============

Raulkumar was kind enough to make a few changes to the README, including
adding commas where needed, capitalizing OTP, among other things in
#552.

Not all the changes were desired, so this commit includes the correct
changes while leaving the other ones out.

Co-authored-by: @Raulkumar
@germsvel
Copy link
Collaborator

Closed by 5e35f5f

@germsvel germsvel closed this Feb 18, 2021
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.

2 participants