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.

Sign up for GitHub

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

Jump to bottom

feat(number): add multipleOf to faker.number.int #2586

Merged
merged 13 commits into from
Mar 13, 2024
Merged

feat(number): add multipleOf to faker.number.int #2586

merged 13 commits into from
Mar 13, 2024

Conversation

matthewmayer
Copy link
Contributor

fix #2584

@matthewmayer matthewmayer requested a review from a team as a code owner December 24, 2023 11:17
@matthewmayer matthewmayer self-assigned this Dec 24, 2023
@matthewmayer matthewmayer added c: feature Request for new feature m: number Something is referring to the number module labels Dec 24, 2023
@codecov Codecov
Copy link

codecov bot commented Dec 24, 2023 •
edited

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.57%. Comparing base (1169a05) to head (3bcbd63).

Additional details and impacted files 
@@            Coverage Diff             @@
##             next    #2586      +/-   ##
==========================================
- Coverage   99.57%   99.57%   -0.01%     
==========================================
  Files        3034     3034              
  Lines      250434   250453      +19     
  Branches      627      986     +359     
==========================================
+ Hits       249375   249380       +5     
- Misses       1030     1073      +43     
+ Partials       29        0      -29
Files Coverage Δ
src/modules/number/index.ts 100.00% <100.00%> (ø)

... and 31 files with indirect coverage changes

ST-DDT
ST-DDT previously approved these changes Dec 24, 2023
View reviewed changes
src/modules/number/index.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT added this to the vAnytime milestone Dec 24, 2023
ST-DDT
ST-DDT reviewed Dec 24, 2023
View reviewed changes
src/modules/number/index.ts Outdated Show resolved Hide resolved
matthewmayer and others added 3 commits December 25, 2023 09:27
@matthewmayer @ST-DDT
Update index.ts
37001d6 
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
@matthewmayer @ST-DDT
Update index.ts
a38b678 
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
@matthewmayer
Merge branch 'next' into next
0def3d2
ST-DDT
ST-DDT reviewed Dec 27, 2023
View reviewed changes
src/modules/number/index.ts Outdated Show resolved Hide resolved
src/modules/number/index.ts Outdated Show resolved Hide resolved
src/modules/number/index.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT mentioned this pull request Jan 18, 2024
Closed
@ST-DDT ST-DDT added p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug labels Feb 15, 2024
@ST-DDT ST-DDT marked this pull request as draft February 15, 2024 16:20
@ST-DDT
Copy link
Member

ST-DDT commented Mar 4, 2024 •
edited

@matthewmayer What is the status on this PR? Can we help you?

@matthewmayer
Copy link
Contributor Author

Yes, I think I need some help implementing your suggestions

ST-DDT
ST-DDT reviewed Mar 11, 2024
View reviewed changes
test/modules/number.spec.ts Outdated Show resolved Hide resolved
test/modules/number.spec.ts Outdated Show resolved Hide resolved
test/modules/number.spec.ts Outdated Show resolved Hide resolved
test/modules/number.spec.ts Outdated Show resolved Hide resolved
src/modules/number/index.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT marked this pull request as ready for review March 11, 2024 23:13
@ST-DDT
Copy link
Member

ST-DDT commented Mar 11, 2024 •
edited

@matthewmayer Does the change look good to you?

Ready for review.

@ST-DDT ST-DDT requested review from a team March 11, 2024 23:15
Shinigami92
Shinigami92 approved these changes Mar 12, 2024
View reviewed changes
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets just assume nobody is checking for the explicit error message in a catch block 😏

@matthewmayer
Copy link
Contributor Author

lets just assume nobody is checking for the explicit error message in a catch block 😏

i think we can assume thats brittle enough to not be considered breaking
https://xkcd.com/1172/

@matthewmayer
Copy link
Contributor Author

@matthewmayer Does the change look good to you?

Ready for review.

looks good for me. i can't approve as its my own PR

@ST-DDT ST-DDT enabled auto-merge (squash) March 13, 2024 12:25
@ST-DDT ST-DDT merged commit 5ef8ef1 into faker-js:next Mar 13, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ST-DDT ST-DDT ST-DDT approved these changes

@Shinigami92 Shinigami92 Shinigami92 approved these changes

Assignees

@matthewmayer matthewmayer

Labels
c: feature Request for new feature m: number Something is referring to the number module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
None yet
Milestone
vAnytime
Development

Successfully merging this pull request may close these issues.

add multipleOf to faker.number.int
3 participants
@matthewmayer @ST-DDT @Shinigami92