add support to send predefined email template #36
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sargue
reviewed
Mar 30, 2021
| @@ -6,7 +6,9 @@ | |||
| /** | |||
| * This class is deprecated. Don't use it. See below for instructions on how to | |||
| * migrate current code. | |||
| * <h3>Migration guide</h3> | |||
| * | |||
There was a problem hiding this comment.
When i run ./gradlew install it fails with this error
Not sure if that my local issue only or anything I can do about it.
sargue
reviewed
Mar 30, 2021
build.gradle
Outdated
| @@ -9,7 +9,7 @@ apply plugin: "jacoco" | |||
|
|
|||
| group 'net.sargue' | |||
| archivesBaseName = 'mailgun' | |||
| version '1.9.2' | |||
| version '1.9.3' | |||
There was a problem hiding this comment.
Version should be 1.10.0 because it changes the API.
sargue
reviewed
Mar 30, 2021
| @@ -225,6 +225,26 @@ public MailBuilder content(Body body) { | |||
| return text(body.text()).html(body.html()); | |||
| } | |||
|
|
|||
| /** | |||
| * Sets the name of the pre saved template that will be used to be sent by Mailgun. | |||
There was a problem hiding this comment.
Please rephrase this line, it's hard to understand.
sargue
reviewed
Mar 30, 2021
| * @param templateName the name of pre saved template on Mailgun | ||
| * @return this builder | ||
| */ | ||
| public MailBuilder template(String templateName){ |
There was a problem hiding this comment.
Please keep existing formatting convetions: space between ) and { at the end of the line.
sargue
reviewed
Mar 30, 2021
| * @param variables custom JSON data to be attached to the message. | ||
| * @return this builder | ||
| */ | ||
| public MailBuilder variables(String variables){ |
sargue
requested changes
Mar 30, 2021
|
Hi,
Thanks for the review!
Requested changes are done. Answered to your questions in comment.
I will duplicate the answer here as well:
*When i run ./gradlew install it fails with this error* *[image: image]
<https://user-images.githubusercontent.com/63702645/113106266-3cf64480-920b-11eb-8bfc-d9670d40bf2a.png>*
*Not sure if that's my local issue only or anything I can do about it
except for changing the formatting to be without </h3> tag. *
Kind regards
…On Tue, Mar 30, 2021 at 7:48 PM Sergi Baila ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Review done. Some questions asked, some changes requested. Awaiting reply.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#36 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/APGAM5JUKFVWTCHVJVZ2FXDTGH6GHANCNFSM4ZYEMSSA>
.
|
|
Thanks for the changes. The javadoc error is really weird. First time I see it. Which JDK are you using to build? |
|
Hi,
Im using Java version „15.0.1“ 2020-10-20.
But I tried Java 8 and it went perfectly well.
Which one do you use?
…On Thu, 1 Apr 2021 at 12:32, Sergi Baila ***@***.***> wrote:
Thanks for the changes.
The javadoc error is really weird. First time I see it.
Which JDK are you using to build?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#36 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/APGAM5MUTINYNSKRZEQMCTDTGQ4SZANCNFSM4ZYEMSSA>
.
|
|
I was using Java 8 but when switching to 11 or 15 I indeed can reproduce the problem. There are also failing steps. And also sigh JFrog is sunsetting bintray/jcenter so I need to switch also those parts. I guess I need to work a bit on the gradle / build part before merging the PR. |
|
Actually, I think it would be better to merge the PR and then work on the upgrade build configuration. |
sargue
approved these changes
Apr 3, 2021
|
Good morning,
Thank you for reviewing and merging!
Sorry, I think I forgot to add a new tag to PR v1.10.1.
Would you be able to do it?
…On Sat, Apr 3, 2021 at 11:20 AM Sergi Baila ***@***.***> wrote:
Merged #36 <#36> into master.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#36 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/APGAM5MBRRSSDKGDMSCSDCTTG3FTZANCNFSM4ZYEMSSA>
.
|
|
I totally forgot, thanks for reminding me.
…---
Sergi Baila
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday 5 April de 2021 a les 9:18, VitaliiBV ***@***.***> wrote:
Good morning,
Thank you for reviewing and merging!
Sorry, I think I forgot to add a new tag to PR v1.10.1.
Would you be able to do it?
On Sat, Apr 3, 2021 at 11:20 AM Sergi Baila ***@***.***>
wrote:
> Merged #36 <#36> into master.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#36 (comment)>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/APGAM5MBRRSSDKGDMSCSDCTTG3FTZANCNFSM4ZYEMSSA>
> .
>
—
You are receiving this because you modified the open/close state.
Reply to this email directly, [view it on GitHub](#36 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AACRP4CSIHPFBWOANPH3IV3THFP5FANCNFSM4ZYEMSSA).
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
in MailContent.java deleted formating tag as
./gradlew installresulted into failed build because of it.