-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
[1673] Ensure mix_case returns at least one lower and one upper case … #1694
[1673] Ensure mix_case returns at least one lower and one upper case … #1694
Conversation
6a329d6
to
db77d36
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests look good to me, and adding both min_alpha and min_numaric makes sense to me.
Oh, and docs need to be updated. If possible it would be nice if you could include Yard style docs since that's in the pipeline, but not required. |
db77d36
to
cac7ab8
Compare
cac7ab8
to
1b9d99c
Compare
Working on the docs now 👍 |
ff690bb
to
2a7af9f
Compare
lib/faker/default/alphanumeric.rb
Outdated
@@ -4,7 +4,8 @@ module Faker | |||
class Alphanumeric < Base | |||
class << self | |||
ALPHABET = ('a'..'z').to_a | |||
ALPHANUMS = ALPHABET + (0..9).to_a | |||
NUMBERS = (0..9).to_a | |||
ALPHANUMS = ALPHABET + NUMBERS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might be redefining/duplicating this constant since Faker::Base
already uses something similar. Check this out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vbrazo good call. I updated to use the constants from the base class.
Faker::Lorem.characters #=> "uw1ep04lhs0c4d931n1jmrspprf5wrj85fefue0y7y6m56b6omquh7br7dhqijwlawejpl765nb1716idmp3xnfo85v349pzy2o9rir23y2qhflwr71c1585fnynguiphkjm8p0vktwitcsm16lny7jzp9t4drwav3qmhz4yjq4k04x14gl6p148hulyqioo72tf8nwrxxcclfypz2lc58lsibgfe5w5p0xv95peafjjmm2frkhdc6duoky0aha" | ||
Faker::Lorem.characters(number: 10) #=> "ang9cbhoa8" | ||
Faker::Lorem.characters(number: 10, min_alpha: 4) #=> "ang9cbhoa8" | ||
Faker::Lorem.characters(number: 10, min_alpha: 4, min_numeric: 1) #=> "ang9cbhoa8" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 👍
97b9cee
to
9e8feb4
Compare
@Zeragamba @vbrazo I believe I have addressed all of your feedback. Let me know if you see anything else 👍 |
9e8feb4
to
76566d9
Compare
@bpleslie could you rebase and fix the conflicts? |
76566d9
to
0e8439f
Compare
0e8439f
to
942091a
Compare
@vbrazo we should be all set here |
* Bump to version 2.2.0 * Add #1694
* Bump to version 2.2.0 * Add faker-ruby#1694
* Bump to version 2.2.0 * Add faker-ruby#1694
…letter
Resolves Issue #1673
Description:
In order to ensure that at least one lowercase and one uppercase letter was returned when passing
mix_case: true
toInternet.password
, I did the following:Alphanumeric.alphanumeric
to acceptmin_alpha
andmin_numeric
params. (Note:min_numeric
is not used, but seemed equally helpful for certain situations. I'm happy to remove it if desired.)min_alpha
inInternet.password
altered themix_case
condition to ensure we always return at least one uppercase and one lowercase letter ifmix_case
istrue
.