-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[1673] Ensure mix_case returns at least one lower and one upper case … #1694
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
Conversation
6a329d6 to
db77d36
Compare
Zeragamba
left a comment
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
| 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: truetoInternet.password, I did the following:Alphanumeric.alphanumericto acceptmin_alphaandmin_numericparams. (Note:min_numericis not used, but seemed equally helpful for certain situations. I'm happy to remove it if desired.)min_alphainInternet.passwordaltered themix_casecondition to ensure we always return at least one uppercase and one lowercase letter ifmix_caseistrue.