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

Fixing compilation warnings #304

Merged
merged 1 commit into from
Aug 22, 2017

Conversation

kelvinst
Copy link
Contributor

@kelvinst kelvinst commented Jul 1, 2017

No description provided.

new_email()
# @user will be available in the template
|> render(:email_with_assigns, user: user)
if Code.ensure_loaded?(Phoenix) do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only line added, the rest is only indentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, there is another way to do it, removing only: :test from phoenix dep declaration. But I found this one a little bit better since bamboo itself does not depend on phoenix.

Going far away in my imagination, I guess it would be a good idea to separate some of Phoenix and Plug code from here. This could be an umbrella project with three applications: bamboo with the basics, bamboo_phoenix to use some phoenix views to render and bambo_viewer for the email viewer.

Will be happy to open a PR to it if you liked the umbrella idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think at some point a bamboo_phoenix lib might be nice. For now this works great. Thank you so much!

@mravey
Copy link

mravey commented Jul 28, 2017

Any chance this get merged?

Copy link
Contributor

@paulcsmith paulcsmith left a comment

Choose a reason for hiding this comment

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

Could you rebase this against master, and when you're done can you ping me? I'll go ahead and merge it once that's done 👍

Thank you!

new_email()
# @user will be available in the template
|> render(:email_with_assigns, user: user)
if Code.ensure_loaded?(Phoenix) do
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think at some point a bamboo_phoenix lib might be nice. For now this works great. Thank you so much!

@kelvinst kelvinst force-pushed the compilation-warnings branch 3 times, most recently from 62aa536 to 7e6052b Compare August 22, 2017 12:09
@kelvinst
Copy link
Contributor Author

Thanks! Rebased!

@paulcsmith paulcsmith merged commit 945fff9 into beam-community:master Aug 22, 2017
@paulcsmith
Copy link
Contributor

Thanks a ton!

paulcsmith pushed a commit that referenced this pull request Nov 8, 2017
This reverts the change in #304 that caused `Bamboo.Phoenix` to never be compiled in Phoenix installations.

Resolves #320
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.

3 participants