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

reginaldo/refactoring_sanctions #28

Open
wants to merge 22 commits into
base: master
from

Conversation

Projects
None yet
7 participants
@tazeek
Copy link
Contributor

tazeek commented Mar 6, 2019

No description provided.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 6, 2019

Codecov Report

Merging #28 into master will increase coverage by 0.48%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #28      +/-   ##
==========================================
+ Coverage   83.85%   84.33%   +0.48%     
==========================================
  Files           2        2              
  Lines         161      166       +5     
  Branches       29       30       +1     
==========================================
+ Hits          135      140       +5     
  Misses         15       15              
  Partials       11       11
Impacted Files Coverage Δ
lib/Data/Validate/Sanctions.pm 84.09% <100%> (+0.95%) ⬆️

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 1e89b39...72bd187. Read the comment docs.

tazeek added some commits Mar 6, 2019

Show resolved Hide resolved README.md Outdated
Show resolved Hide resolved lib/Data/Validate/Sanctions.pm Outdated
Show resolved Hide resolved lib/Data/Validate/Sanctions.pm Outdated

zakame and others added some commits Mar 13, 2019

Remove extras
Co-Authored-By: tazeek <tazeek.rakib@gmail.com>
Ditto
Co-Authored-By: tazeek <tazeek.rakib@gmail.com>
README updated
Co-Authored-By: tazeek <tazeek.rakib@gmail.com>

tazeek and others added some commits Mar 14, 2019

my @cleaned_full_name = map { s/[^[:alpha:]\s]/ /gr } split ' ', $full_name;

# Remove trailing and leading whitespaces
@cleaned_full_name = map { s/^\s*(.*)\s*$/$1/g; split ' ', uc } @cleaned_full_name;

This comment has been minimized.

@tfoertsch

tfoertsch Mar 18, 2019

this does not work as expected. Sorry for missing this the last round.

# perl -le 'print ">" . (s/^\s*(.*)\s*$/$1/gr) . "<" for (@ARGV)' '   fasd     ' fads
>fasd     <
>fads<

As you can see, leading spaces are removed but trailing spaces are kept. The reason is the * in .* is greedy. It should work as expected if you change the regex to ^\s*(.*?)\s*$. *? is the non-greedy version.

This comment has been minimized.

@tfoertsch

tfoertsch Mar 18, 2019

Maybe you want to change this to

# perl -le 'print ">$_<" for (map { split q{ }, uc s/^\s*(.*?)\s*$/$1/gr } @ARGV)' '   fasd fa fad     ' fads
>FASD<
>FA<
>FAD<
>FADS<

This might even resolve the perlcritic complaint.

This comment has been minimized.

@binarytom

binarytom Mar 18, 2019

Contributor

Good catch. I'd suggest leaving out the regex entirely, since there isn't going to be any leading/trailing whitespace anyway:

perl -le 'print ">$_<" for (map { split " ", uc } @ARGV)' '   fasd fa fad

This would also be simple enough to combine with the previous step so that you're doing

my @cleaned_full_name = split " ", $full_name =~ s/[^[:alpha:]\s]/ /gr;

which should help avoid ending up with empty elements in the array, e.g. Fréderico é Estrela or Alan Ó Néill.

(search for awk in https://perldoc.pl/functions/split for details)

tazeek added some commits Mar 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.