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.

Sign up for GitHub

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

Jump to bottom

Stdlib::Email type #1160

Merged
merged 1 commit into from
Feb 8, 2021
Merged

Stdlib::Email type #1160

merged 1 commit into from
Feb 8, 2021

Conversation

b4ldr
Copy link
Collaborator

@b4ldr b4ldr commented Feb 4, 2021

Add new type to validate emails. The regex has been taken from the
html5 spec

@b4ldr b4ldr requested a review from a team as a code owner February 4, 2021 10:40
@b4ldr
Stdlib::Email type
ea1557b 
Add new type to validate emails.  The regex has been taken from the
[html5 spec][1]

[1]: https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address
@codecov-io
Copy link

codecov-io commented Feb 4, 2021 •
edited
Loading

Codecov Report

Merging #1160 (ea1557b) into main (57b339a) will increase coverage by 0.86%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##            main   #1160      +/-   ##
========================================
+ Coverage   3.69%   4.56%   +0.86%     
========================================
  Files        185     185              
  Lines       5191    5172      -19     
========================================
+ Hits         192     236      +44     
+ Misses      4999    4936      -63
Impacted Files Coverage Δ
lib/puppet/parser/functions/type3x.rb 0.00% <0.00%> (-72.23%) ⬇️
lib/facter/root_home.rb 65.38% <0.00%> (-3.85%) ⬇️
lib/puppet/type/file_line.rb 96.36% <0.00%> (+1.81%) ⬆️
lib/facter/package_provider.rb 85.71% <0.00%> (+28.57%) ⬆️
lib/puppet/provider/file_line/ruby.rb 62.62% <0.00%> (+47.47%) ⬆️
lib/puppet/parser/functions/dig44.rb 53.33% <0.00%> (+53.33%) ⬆️

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 57b339a...ea1557b. Read the comment docs.

@pmcmaw pmcmaw added the feature label Feb 8, 2021
@pmcmaw
Copy link
Contributor

pmcmaw commented Feb 8, 2021

Thanks for your PR @b4ldr :-)

@pmcmaw pmcmaw merged commit 3b5f268 into puppetlabs:main Feb 8, 2021
@pegasd
Copy link
Contributor

pegasd commented Feb 10, 2021

I just noticed this PR (apparently a little late) and see that newline handling is not done properly here.
As I mention here #1066, it's better to use \A, \z instead of ^, $ in all type aliases because of the way multiline strings are handled in Puppet.

Currently, things like

random stuff
valid@email.com
more random stuff $^*!

will match, which is, obviously, not correct.

@pmcmaw
Copy link
Contributor

pmcmaw commented Feb 10, 2021

@pegasd thanks for bringing this to my attention.

Please note nothing is ever too late :-) Our code bases are always evolving and changes/PRs can be merged at anytime.
I have opened the following PR to resolve the issue you have highlighted and I would be super grateful if you had time for a review when the tests run.

PR to resolve: #1163

@pmcmaw pmcmaw mentioned this pull request Feb 10, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@b4ldr @codecov-io @pmcmaw @pegasd