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

Provide a way of limiting the mail storage size #237

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kjagiello
Copy link

MailDev may sometimes be used as a part of a development/staging environment where the process may be running for a very long time without interruption. In those cases, it is undesirable to let that
process grow freely. This limit makes sure that the memory usage is bounded.

@codecov
Copy link

codecov bot commented Nov 15, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@0ff1f28). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #237   +/-   ##
=========================================
  Coverage          ?   65.37%           
=========================================
  Files             ?        9           
  Lines             ?      517           
  Branches          ?      109           
=========================================
  Hits              ?      338           
  Misses            ?      179           
  Partials          ?        0
Impacted Files Coverage Δ
lib/mailserver.js 66.98% <100%> (ø)
index.js 72.5% <100%> (ø)

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 0ff1f28...a7b1ce6. Read the comment docs.

MailDev may sometimes be used as a part of a development/staging
environment where the process may be running for a very long time
without interruption. In those cases, it is undesirable to let that
process grow freely. This limit makes sure that the memory usage is
bounded.
@kgeis
Copy link
Collaborator

kgeis commented Nov 17, 2018

This is a great feature, and I am looking forward to using it.

Thank you for including a test. Here's what I'd like to see before pulling:

  • Update the Usage section of README.md to document the new option.
  • Apply the setting with a setter like how autoRelay is done. I see this as a less-used property of mailServer, and I don't want an explosion of parameters on the create(..) method.
  • Remove repetition of email count in the test (one instance of 2, two instances of 3, and transporter.sendMail() being called three times).

@kjagiello
Copy link
Author

@kgeis

  • Update the Usage section of README.md to document the new option.
  • Apply the setting with a setter like how autoRelay is done. I see this as a less-used property of mailServer, and I don't want an explosion of parameters on the create(..) method.

Good points! Fixed!

  • Remove repetition of email count in the test (one instance of 2, two instances of 3, and transporter.sendMail() being called three times).

I did find some flaws in the test, so I have redone it now. Take a look at it and let me know you see any more issues there.

@kjagiello
Copy link
Author

Also, as you can see in the test, I had to clear the store, because it contained leftovers from the previous test cases. We should probably empty the store every time a new EmailServer is created (under the assumption that there should be max one running instance of it)?

@kjagiello
Copy link
Author

@kgeis Do you think we could this merged in sometime soon?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants