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

feat: throw error on unknown locale #1071

Merged
merged 1 commit into from Jun 14, 2022
Merged

Conversation

Shinigami92
Copy link
Member

No description provided.

@Shinigami92 Shinigami92 added the c: feature Request for new feature label Jun 14, 2022
@Shinigami92 Shinigami92 added this to the v7 - Current Major milestone Jun 14, 2022
@Shinigami92 Shinigami92 requested a review from a team as a code owner June 14, 2022 09:14
@Shinigami92 Shinigami92 self-assigned this Jun 14, 2022
@codecov
Copy link

codecov bot commented Jun 14, 2022

Codecov Report

Merging #1071 (0fe2201) into main (e8985f6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1071   +/-   ##
=======================================
  Coverage   99.64%   99.64%           
=======================================
  Files        2135     2135           
  Lines      229134   229160   +26     
  Branches      970      978    +8     
=======================================
+ Hits       228326   228354   +28     
+ Misses        787      785    -2     
  Partials       21       21           
Impacted Files Coverage Δ
src/faker.ts 100.00% <100.00%> (ø)
src/modules/internet/user-agent.ts 86.95% <0.00%> (+0.57%) ⬆️

@ST-DDT ST-DDT requested review from a team June 14, 2022 09:21
@ST-DDT ST-DDT added p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug labels Jun 14, 2022
@xDivisionByZerox
Copy link
Member

Should this be marked as breaking change? As far as I can tell this can lead to unhandled errors on the end-user side.

@Shinigami92
Copy link
Member Author

Should this be marked as breaking change? As far as I can tell this can lead to unhandled errors on the end-user side.

I would say no, because otherwise it would should just fail later when accessing a function that depends on it as you would get a "null-pointer error"
This brings the error just a little bit close upfront and you know whats going on

@Shinigami92 Shinigami92 merged commit 5ea8252 into main Jun 14, 2022
@Shinigami92 Shinigami92 deleted the throw-error-on-unknown-locale branch June 14, 2022 12:18
Minozzzi pushed a commit to Minozzzi/faker that referenced this pull request Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants