Skip to content

Add MailAddress.TryCreate api - #1052

Merged
davidsh merged 8 commits into
dotnet:masterfrom
MarcoRossignoli:tryparsemailaddress
Feb 10, 2020
Merged

davidsh merged 8 commits into
dotnet:masterfrom
MarcoRossignoli:tryparsemailaddress

Conversation

@MarcoRossignoli

Copy link
Copy Markdown
Member

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

opted for tuple because is used only inside this class

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've reorganized data to allow reuse and equality with ctors tests.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the only place where we need to rely on exception handling.

@MarcoRossignoli

Copy link
Copy Markdown
Member Author

/azp list

@azure-pipelines

Copy link
Copy Markdown
Commenter does not have sufficient privileges for PR 1052 in repo dotnet/runtime

@scalablecory
scalablecory requested a review from a team December 31, 2019 13:50
@scalablecory scalablecory added this to the 5.0 milestone Dec 31, 2019
@MarcoRossignoli

Copy link
Copy Markdown
Member Author

/azp list

@azure-pipelines

Copy link
Copy Markdown
Commenter does not have sufficient privileges for PR 1052 in repo dotnet/runtime

@MihaZupan

Copy link
Copy Markdown
Member

@ViktorHofer Shouldn't the external-ci group have azp permissions?

@MarcoRossignoli

Copy link
Copy Markdown
Member Author

just asked #718 (comment)

@vcsjones

Copy link
Copy Markdown
Member

/azp list

@azure-pipelines

Copy link
Copy Markdown
Commenter does not have sufficient privileges for PR 1052 in repo dotnet/runtime

Comment thread src/libraries/Common/src/System/Net/Mail/MailAddressParser.cs Outdated
Comment thread src/libraries/Common/src/System/Net/Mail/DotAtomReader.cs Outdated
Comment thread src/libraries/Common/src/System/Net/Mail/MailAddressParser.cs Outdated
Comment thread src/libraries/Common/src/System/Net/Mail/MailAddressParser.cs Outdated
Comment thread src/libraries/Common/src/System/Net/Mail/MailAddressParser.cs Outdated

@stephentoub stephentoub left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left a few comments, but generally LGTM.

@stephentoub

Copy link
Copy Markdown
Member

@dotnet/ncl, can someone please review?

Comment thread src/libraries/System.Net.Mail/src/System/Net/Mail/MailAddress.cs Outdated
Comment thread src/libraries/System.Net.Mail/src/System/Net/Mail/MailAddress.cs Outdated
Comment thread src/libraries/Common/src/System/Net/Mail/DomainLiteralReader.cs Outdated
Comment thread src/libraries/Common/src/System/Net/Mail/DotAtomReader.cs Outdated
Comment thread src/libraries/Common/src/System/Net/Mail/MailAddressParser.cs Outdated
Comment thread src/libraries/System.Net.Mail/src/System/Net/Mail/MailAddress.cs Outdated
Comment thread src/libraries/System.Net.Mail/src/System/Net/Mail/MailAddress.cs Outdated
Comment thread src/libraries/System.Net.Mail/src/System/Net/Mail/MailAddress.cs Outdated

@davidsh davidsh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks for the PR!

@davidsh
davidsh merged commit aea45f4 into dotnet:master Feb 10, 2020
@MarcoRossignoli
MarcoRossignoli deleted the tryparsemailaddress branch February 11, 2020 07:48
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TryParse for email addresses

9 participants