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: faker.finance.pin() #695

Merged
merged 13 commits into from
Apr 6, 2022
Merged

feat: faker.finance.pin() #695

merged 13 commits into from
Apr 6, 2022

Conversation

import-brain
Copy link
Member

fixes #472

@import-brain import-brain added c: feature Request for new feature p: 1-normal Nothing urgent labels Mar 27, 2022
@import-brain import-brain requested a review from a team March 27, 2022 14:38
@import-brain import-brain requested a review from a team as a code owner March 27, 2022 14:38
@import-brain import-brain self-assigned this Mar 27, 2022
@codecov
Copy link

codecov bot commented Mar 27, 2022

Codecov Report

Merging #695 (a8abe45) into main (5beac4b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

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

@@           Coverage Diff           @@
##             main     #695   +/-   ##
=======================================
  Coverage   99.34%   99.34%           
=======================================
  Files        1921     1921           
  Lines      176656   176673   +17     
  Branches      905      909    +4     
=======================================
+ Hits       175503   175520   +17     
  Misses       1097     1097           
  Partials       56       56           
Impacted Files Coverage Δ
src/finance.ts 99.33% <100.00%> (+0.02%) ⬆️

test/finance.spec.ts Outdated Show resolved Hide resolved
src/finance.ts Outdated Show resolved Hide resolved
src/finance.ts Outdated Show resolved Hide resolved
src/finance.ts Outdated Show resolved Hide resolved
@xDivisionByZerox
Copy link
Member

What would be the expected behavior when a negative length is requested?

@import-brain
Copy link
Member Author

What would be the expected behavior when a negative length is requested?

I didn't consider that scenario. Do you think it should throw an error?

Shinigami92
Shinigami92 previously approved these changes Mar 27, 2022
@pkuczynski
Copy link
Member

What would be the expected behavior when a negative length is requested?

I didn't consider that scenario. Do you think it should throw an error?

I personally think it should throw when < 1...

src/finance.ts Show resolved Hide resolved
test/finance.spec.ts Outdated Show resolved Hide resolved
Co-authored-by: Piotr Kuczynski <[email protected]>
src/finance.ts Outdated Show resolved Hide resolved
Co-authored-by: Piotr Kuczynski <[email protected]>
@import-brain import-brain requested review from a team and removed request for Shinigami92 March 27, 2022 18:19
src/finance.ts Outdated Show resolved Hide resolved
src/finance.ts Show resolved Hide resolved
ST-DDT
ST-DDT previously approved these changes Mar 27, 2022
pkuczynski
pkuczynski previously approved these changes Mar 27, 2022
src/finance.ts Outdated Show resolved Hide resolved
@Shinigami92 Shinigami92 enabled auto-merge (squash) April 6, 2022 07:51
@Shinigami92 Shinigami92 merged commit 20f33e6 into main Apr 6, 2022
@Shinigami92 Shinigami92 deleted the feat_pin branch April 6, 2022 07:57
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PIN, account number
5 participants