[go: nahoru, domu]

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

Feature html color #15

Closed
wants to merge 3 commits into from
Closed

Feature html color #15

wants to merge 3 commits into from

Conversation

jlannoy
Copy link
Contributor
@jlannoy jlannoy commented May 31, 2017

It was noticed it in the comment of the private tag() method, but not
really possible to do. Now it is ;)

It was noticed it in the comment of the private tag() method, but not
really possible to do. Now it is ;)
Copy link
Owner
@sargue sargue 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 the PR. Please adjust the javadocs.

* version will get only the content.
*
* @param content the text content
* @param content the text content
Copy link
Owner

Choose a reason for hiding this comment

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

Duplicated @param content, should be color.

@jlannoy
Copy link
Contributor Author
jlannoy commented May 31, 2017

Sorry, I duplicated the param but forgot to change it... Done.

@sargue
Copy link
Owner
sargue commented May 31, 2017

I'm having a second though about this. Reviewing the Builder class I see there is no other method like the one you propose. It seems a bit ad-hoc to me.

Maybe a better solution would be just to make the tag method public.

@sargue
Copy link
Owner
sargue commented May 31, 2017

I'm publishing the tag method. See this: https://github.com/sargue/mailgun/wiki/Mail-content-using-content-helpers#low-level-html

Do you think that's enough?

@jlannoy
Copy link
Contributor Author
jlannoy commented May 31, 2017

I completely agree with you. I didn't want to break the public/private scope of your methods but it's more efficient in this way.

Thank you.

@sargue
Copy link
Owner
sargue commented May 31, 2017

Great. I'm closing this PR and publishing a new version.

@sargue sargue closed this May 31, 2017
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.

None yet

2 participants