Skip to content

Add Parse Server Generic Email Adapter to README #4101

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

Merged
merged 10 commits into from
Dec 19, 2017

Conversation

premithk
Copy link
Contributor

Added a generic email plugin to send Parse Server password reset and verification emails with generic accounts like Gmail. This module uses the nodemailer library.

Added a generic email plugin to send Parse Server password reset and verification emails with generic accounts like Gmail. This module uses the nodemailer library.
@codecov
Copy link

codecov bot commented Aug 18, 2017

Codecov Report

Merging #4101 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4101      +/-   ##
==========================================
- Coverage   92.69%   92.67%   -0.03%     
==========================================
  Files         118      118              
  Lines        8353     8353              
==========================================
- Hits         7743     7741       -2     
- Misses        610      612       +2
Impacted Files Coverage Δ
src/RestWrite.js 93.28% <0%> (-0.55%) ⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 96.94% <0%> (+0.1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bad2179...e3bc809. Read the comment docs.

@addisonElliott
Copy link
Contributor

What's the status on this PR?

README.md Outdated
@@ -310,7 +310,7 @@ You can also use other email adapters contributed by the community such as:
- [parse-server-mailgun-adapter-template](https://www.npmjs.com/package/parse-server-mailgun-adapter-template)
- [parse-server-mailjet-adapter](https://www.npmjs.com/package/parse-server-mailjet-adapter)
- [simple-parse-smtp-adapter](https://www.npmjs.com/package/simple-parse-smtp-adapter)

- [parse-server-genericemail-adapter](https://www.npmjs.com/package/parse-server-genericemail-adapter)
Copy link
Contributor

Choose a reason for hiding this comment

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

you need an empty line after that please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, have added the empty line

Have added the empty line
@premithk
Copy link
Contributor Author

Have made the changes

@montymxb montymxb changed the title Update README.md Add Parse Server Generic Email Adapter to README Nov 28, 2017
@montymxb
Copy link
Contributor

@premithk The adapter sounds nice, but to be honest I don't feel this project is at a point where we should endorse it on our page. It indicates it was last tested against parse-server 2.2.13 and the README could use some work.

If you wrote this you can check out the other adapters to see a good example of some solid READMEs. Yours doesn't have to copy or anything, but it should be something professional that we would be happy to have as a link to on our page. Additionally, and maybe this can't be changed, but the name is missing a hyphen where I would expect parse-server-generic-email-adapter.

Not trying to bash on the adapter though, what it does seems quite useful.

@premithk
Copy link
Contributor Author

@montymxb thank you, i see your point, let me test it against the latest parse-server and update the readme. Regarding the missing hyphen i don't know if i can change that :(

@premithk
Copy link
Contributor Author

@montymxb have updated the readme, can you please check and see if that is enough. (At your leisure). Thank you

@montymxb
Copy link
Contributor

@premithk thanks for the changes, there were some additional things I noticed on the repo still. I'll open up a PR there when I get a chance.

@premithk
Copy link
Contributor Author

premithk commented Dec 1, 2017

Sure, thank you! :)

@montymxb
Copy link
Contributor

montymxb commented Dec 8, 2017

@premithk I'm going to take a look at your repo to suggest some changes.

On another note regarding the package name, if you're willing to we would like to have the name cleaned up as mentioned above. You could publish a new npm package and deprecate the old one. This has been done before and there's a deprecation guide for npm modules as well as a little discussion on it in this SO post. It's asking a bit, but it would be nice 😄 .

@premithk
Copy link
Contributor Author

premithk commented Dec 8, 2017

@montymxb thank you for the pull request, i've merged those. lemme try and clean up the npm name issue

@premithk
Copy link
Contributor Author

premithk commented Dec 8, 2017

@montymxb i have deprecated the old funky name one and published a new npm package with the above mentioned name https://www.npmjs.com/package/parse-server-generic-email-adapter. Also have updated the readme with the package name changes. Please check at your leisure.

Copy link
Contributor

@montymxb montymxb left a comment

Choose a reason for hiding this comment

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

Alright the package looks much better now. One more thing though, you need to update all instances of parse-server-genericemail-adapter in your readme to the new package name as well. Once that's done, ,and it's been republishd on npm so the README is corrected, let me know.

@premithk
Copy link
Contributor Author

@montymxb have updated the readme with the new package names and republished on npm. Thank you.

@montymxb
Copy link
Contributor

montymxb commented Dec 12, 2017 via email

Copy link
Contributor

@montymxb montymxb left a comment

Choose a reason for hiding this comment

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

Alright I would say this is approved :octocat: .

Tested this against a little fake gmail account I had, and it works! Was pleasantly surprised to see it send with very little work.

The one thing I did note was a deprecation notice regarding the version of nodemailer you're using.

WARN deprecated [email protected]: All versions below 4.0.1 of Nodemailer are deprecated. See https://nodemailer.com/status/

Since it's just a deprecation this is acceptable for the moment, but I would address it by bumping up the version when you have time.

As a final note, although I'm giving this approval, I would like to get one more word from @flovilmart to see if he has any last thoughts or concerns.

@premithk
Copy link
Contributor Author

@montymxb have merged your changes. thank you

@montymxb montymxb dismissed flovilmart’s stale review December 19, 2017 18:10

requested change was made

@montymxb
Copy link
Contributor

@premithk just rerunning CI to so we can bring this in. Since the requested changes have been made we're definitely good to go.

@montymxb montymxb merged commit 33de770 into parse-community:master Dec 19, 2017
@montymxb
Copy link
Contributor

Thanks again @premithk !

@premithk
Copy link
Contributor Author

Thanks a lot @montymxb

flovilmart pushed a commit that referenced this pull request Dec 29, 2017
Added a generic email plugin to send Parse Server password reset and verification emails with generic accounts like Gmail. This module uses the nodemailer library.
UnderratedDev pushed a commit to UnderratedDev/parse-server that referenced this pull request Mar 21, 2020
Added a generic email plugin to send Parse Server password reset and verification emails with generic accounts like Gmail. This module uses the nodemailer library.
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.

4 participants