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

[GH-40] Add property type email #84

Merged
merged 1 commit into from
Mar 18, 2021
Merged

[GH-40] Add property type email #84

merged 1 commit into from
Mar 18, 2021

Conversation

renjithgr
Copy link
Contributor

Summary

This PR adds the option to choose email as a field type.

image

Ticket Link

#40

Most of the components are not covered by unit tests, including the ones I changed. So I was not sure what strategy the creators have in mind for unit testing components. Please advise. I would be happy to add unit tests.

Thank you!

@mattermod
Copy link
Contributor

Hello @renjithgr,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

@renjithgr renjithgr changed the title Add property type email [GH-40] Add property type email Mar 18, 2021
@jespino jespino self-requested a review March 18, 2021 07:51
@jespino
Copy link
Contributor

jespino commented Mar 18, 2021

@renjithgr unit tests are really welcome, we want to have a good test coverage, we have implemented this without unit testing to move fast in this stage of the process, but we will add them for sure sooner than later.

@renjithgr
Copy link
Contributor Author

Thanks for your feedback, @jespino. Let me know if you want me to add unit tests in this PR or you want to go ahead without unit tests for now. I would be happy to raise another PR to add unit tests.

@jespino
Copy link
Contributor

jespino commented Mar 18, 2021

@renjithgr Lets merge this in, and add the unit tests in another PR. :)

Copy link
Contributor

@jespino jespino left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks @renjithgr! 🎉

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

Successfully merging this pull request may close these issues.

3 participants