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

sort mx records by priority (lowest first) #57

Merged
merged 1 commit into from
Jul 4, 2019

Conversation

seasick
Copy link
Contributor

@seasick seasick commented Oct 9, 2018

Taken from https://en.wikipedia.org/wiki/MX_record#Priority

Mail is delivered to the mail exchange server with the lowest preference number (highest priority), so the MX record you use for mail routing should have the lowest preference number, typically 0.

Description

Currently the mail is sent by using the MX record with the highest priority. This PR changes it to use the record with the lowest priority first.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@GreenPioneer
Copy link
Collaborator

GreenPioneer commented Oct 15, 2018

@seasick thank you for the pr - anything else you see that needs work?

I will push within the next couple of days

@GreenPioneer GreenPioneer self-requested a review October 15, 2018 01:46
@GreenPioneer GreenPioneer added this to the 1.x milestone Oct 15, 2018
@seasick
Copy link
Contributor Author

seasick commented Oct 15, 2018

@GreenPioneer, you are welcome. Noticed it, though it is an easy fix. Even without that fix, it works like a charm :)

@luisesn
Copy link

luisesn commented Jun 17, 2019

No news from this?

@benbucksch
Copy link

benbucksch commented Jul 3, 2019

9 months ago:

I will push within the next couple of days

@GreenPioneer : This PR is really important. As-is, you will always send to the wrong SMTP server, if there are backup fallback servers.

@GreenPioneer GreenPioneer merged commit 9be64c8 into guileen:master Jul 4, 2019
@GreenPioneer
Copy link
Collaborator

@seasick thanks again and my bad on it taking so dang long to get it in

I don't know how I wasn't notified by @luisesn message but I was with @benbucksch message

Will be publishing to NPM momentarily

@GreenPioneer
Copy link
Collaborator

https://www.npmjs.com/package/sendmail/v/1.5.0

@benbucksch
Copy link

@GreenPioneer : Thanks!

@luisesn
Copy link

luisesn commented Jul 4, 2019

Thanks!

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.

4 participants