Closed amitmawkin closed 7 years ago
@nelsonic please review and Merge , hopefully got it correct this time.
@amitmawkin I don't understand why test/code coverage needs to go down... 🤔 any chance you can add a test that exercise the 2 lines being missed?
I did, if you go to codecov now, it should show 100%.. BTW I need to add some more code, as I just came to know that AWS only allows 50 email addresses in one email irrespective of the fields. So i will push the new code tonight
Merging #65 into master will not change coverage. The diff coverage is
100%
.
@@ Coverage Diff @@
## master #65 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 1 1
Lines 29 42 +13
=====================================
+ Hits 29 42 +13
Impacted Files | Coverage Δ | |
---|---|---|
lib/index.js | 100% <100%> (ø) |
:white_check_mark: |
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 4e43467...b2e3617. Read the comment docs.
Sure thing @nelsonic will try to get these in, reason for new method was to avoid breaking the code for backwards compatibility, as this was the first time putting the code in . You were right for the indent, i use vscode and the format plugin was using 4 spaces instead of two. i have fixed that.
I will also update the Readme and put some examples :)
proxy was used if you are running in a corporate environment for local testing , I can take it out not a big thing , i will have to test it out if it works without it when developing locally on my mac
Hi @amitmawkin and @nelsonic. The changes in this PR are useful for my project so I'm gonna make the suggested review changes for this PR. Hope this is ok!
@roryc89 great plan! thanks! ❤️
Sure .. i can review it also. I was travelling so could not finish it after @nelsonic review. Let me know if you can wait a day or two or go ahead if it is urge mnt
Thanks @amitmawkin for the work - it's a big help!
I'm afraid the deadline for our project is somewhat pressing.
I've already resolved the conflicts and made some of the suggested review changes so I could push this and you could check that the changes I made are acceptable for you?
@nelsonic and @amitmawkin I've updated the code with the changes suggested in the review. Please let me if there's anything you'd prefer done differently! :)
@roryc89 / @shouston3 what version are we releasing this as? 3.2 or 4.0?
@nelsonic I thought all my changes were backwards compatible so I don't know any reason to bump to 4
@roryc89 yeah, looks backward compatible to me too. so thinking 3.2.0
?
sendemail@3.2.0
on NPM
cc: @amitmawkin 🚀