Skip to content

fix: alphaNumeric all chars banned#550

Merged
Shinigami92 merged 4 commits into
faker-js:mainfrom
xDivisionByZerox:fix/alpha-numeric-all-chars-banned
Mar 21, 2022
Merged

fix: alphaNumeric all chars banned#550
Shinigami92 merged 4 commits into
faker-js:mainfrom
xDivisionByZerox:fix/alpha-numeric-all-chars-banned

Conversation

@xDivisionByZerox

@xDivisionByZerox xDivisionByZerox commented Feb 23, 2022

Copy link
Copy Markdown
Member

Created in relation to #518.

  • remove additional if check against options argument
  • throw if no characters to pick from are available
  • activate test case
@xDivisionByZerox xDivisionByZerox requested a review from a team as a code owner February 23, 2022 19:46
@codecov

codecov Bot commented Feb 23, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #550 (e6515c9) into main (5a15c40) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head e6515c9 differs from pull request most recent head b837232. Consider uploading reports for the commit b837232 to get more accurate results

@@           Coverage Diff           @@
##             main     #550   +/-   ##
=======================================
  Coverage   99.33%   99.33%           
=======================================
  Files        1920     1920           
  Lines      176482   176489    +7     
  Branches      905      908    +3     
=======================================
+ Hits       175302   175309    +7     
  Misses       1124     1124           
  Partials       56       56
Impacted Files Coverage Δ
src/random.ts 99.22% <100.00%> (+<0.01%) ⬆️
src/helpers.ts 99.10% <0.00%> (-0.03%) ⬇️
src/iban.ts 100.00% <0.00%> (ø)
@Shinigami92 Shinigami92 added the p: 2-high Fix main branch label Feb 23, 2022
@Shinigami92 Shinigami92 added this to the v6.1 - First bugfixes milestone Feb 23, 2022
ST-DDT
ST-DDT previously approved these changes Feb 23, 2022
View reviewed changes
@ST-DDT ST-DDT requested a review from a team February 23, 2022 23:10
@ejcheng ejcheng added the c: bug Something isn't working label Feb 24, 2022
@xDivisionByZerox xDivisionByZerox mentioned this pull request Feb 26, 2022
Closed
@ST-DDT ST-DDT linked an issue Feb 26, 2022 that may be closed by this pull request
Unexpected behavior of alphaNumeric with all chars banned #518
Closed
Shinigami92
Shinigami92 reviewed Mar 10, 2022
View reviewed changes
Comment thread src/random.ts Outdated
Comment thread src/random.ts Outdated
@Shinigami92 Shinigami92 added p: 1-normal Nothing urgent and removed p: 2-high Fix main branch labels Mar 15, 2022
@ST-DDT ST-DDT added the s: accepted Accepted feature / Confirmed bug label Mar 21, 2022
@Shinigami92 Shinigami92 enabled auto-merge (squash) March 21, 2022 13:07
@Shinigami92 Shinigami92 merged commit c51fb15 into faker-js:main Mar 21, 2022
@xDivisionByZerox xDivisionByZerox deleted the fix/alpha-numeric-all-chars-banned branch April 15, 2022 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: bug Something isn't working p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug

4 participants

@xDivisionByZerox @ST-DDT @Shinigami92 @ejcheng