Skip to content

Call constructor with an email inside unit test.#349

Closed
dlemstra wants to merge 1 commit intosendgrid:masterfrom
dlemstra:Email
Closed

Call constructor with an email inside unit test.#349
dlemstra wants to merge 1 commit intosendgrid:masterfrom
dlemstra:Email

Conversation

@dlemstra
Copy link
Copy Markdown

This patch makes sure that the other constructor of Email is called during the unit test. This should be merged before pull-request #345 that will fail on this test. The setter of Name should be changed to something like the code below by @stevensona inside his pull request:

set
{
  _name = value?.Trim(Quote);
}

p.s. I don't think we need a CLA for such a minor change in your unit tests or do we?

@thinkingserious thinkingserious added type: community enhancement feature request not on Twilio's roadmap status: cla needed status: work in progress Twilio or the community is in the process of implementing labels Oct 15, 2016
@thinkingserious
Copy link
Copy Markdown
Contributor

Hi @dlemstra,

Unfortunately, we do need the CLA. If you don't mind, please sign it for us. Hopefully, there will be other contributions you will make :)

@stevensona
Copy link
Copy Markdown

Thanks @dlemstra for pointing that out. I missed the fact that in that second form of the constructor, the default value of name is null. That does seems a little weird to me though. Why not have a default value of ""?

@dlemstra
Copy link
Copy Markdown
Author

I found the issue because of the hacktoberfest and thought I could help out a bit. But I am not going to share my personal details for a simple change to your unit tests. Feel free to add my patch to your commits @stevensona. I am going to close this pull request.

@dlemstra dlemstra closed this Oct 15, 2016
@thinkingserious
Copy link
Copy Markdown
Contributor

@dlemstra,

Understood and thanks for graciously offering up your changes to @stevensona. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: work in progress Twilio or the community is in the process of implementing type: community enhancement feature request not on Twilio's roadmap

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants