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(locale): add unionpay module for zh_CN #2338

Merged
merged 24 commits into from Sep 23, 2023
Merged

feat(locale): add unionpay module for zh_CN #2338

merged 24 commits into from Sep 23, 2023

Conversation

CoolPlayLin
Copy link
Contributor

@CoolPlayLin CoolPlayLin commented Aug 23, 2023 •
edited

@CoolPlayLin CoolPlayLin requested a review from a team as a code owner August 23, 2023 10:48
@codecov
Copy link

codecov bot commented Aug 23, 2023 •
edited

Codecov Report

Merging #2338 (192eb73) into next (48a7f54) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files 
@@           Coverage Diff           @@
##             next    #2338   +/-   ##
=======================================
  Coverage   99.61%   99.61%           
=======================================
  Files        2801     2802    +1     
  Lines      252476   252486   +10     
  Branches     1105     1107    +2     
=======================================
+ Hits       251500   251516   +16     
+ Misses        949      943    -6     
  Partials       27       27
Files Changed Coverage Δ
src/locales/zh_CN/finance/credit_card/index.ts 100.00% <100.00%> (ø)
src/locales/zh_CN/finance/credit_card/unionpay.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@ST-DDT ST-DDT added p: 1-normal Nothing urgent c: locale Permutes locale definitions m: finance Something is referring to the finance module labels Aug 26, 2023
@CoolPlayLin
Copy link
Contributor Author

I don't know why test fails

@ST-DDT
Copy link
Member

ST-DDT commented Sep 1, 2023

creditCardNumber() > should generate a valid union pay credit card

I asked the original author of the validatorjs PR for more information:

@ST-DDT ST-DDT added the do NOT merge yet Do not merge this PR into the target branch yet label Sep 1, 2023
@CoolPlayLin CoolPlayLin removed their assignment Sep 1, 2023
@ST-DDT ST-DDT marked this pull request as draft September 1, 2023 21:05
@CoolPlayLin CoolPlayLin changed the title feat(locale): add finance module for zh_CN feat(locale): add unionpay module for zh_CN Sep 1, 2023
@CoolPlayLin
Copy link
Contributor Author

CoolPlayLin commented Sep 10, 2023 •
edited

New replied in

I'm running pnpm run preflight now

matthewmayer
matthewmayer previously approved these changes Sep 16, 2023
View reviewed changes
@CoolPlayLin
Copy link
Contributor Author

@ST-DDT It is able to remove do NOT merge yet now 🤔

@ST-DDT ST-DDT added needs rebase There is a merge conflict and removed do NOT merge yet Do not merge this PR into the target branch yet labels Sep 17, 2023
@ST-DDT
Copy link
Member

ST-DDT commented Sep 17, 2023

Please rebase this PR to include the latest changes.

import-brain
import-brain previously approved these changes Sep 17, 2023
View reviewed changes
@CoolPlayLin
Copy link
Contributor Author

done

ST-DDT
ST-DDT approved these changes Sep 23, 2023
View reviewed changes
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

Assuming the validator is correct, this looks good to me.

@ST-DDT ST-DDT requested review from a team September 23, 2023 08:53
@ST-DDT ST-DDT removed the needs rebase There is a merge conflict label Sep 23, 2023
@ST-DDT ST-DDT enabled auto-merge (squash) September 23, 2023 17:33
@ST-DDT ST-DDT merged commit 74eeccc into faker-js:next Sep 23, 2023
38 checks passed
@CoolPlayLin CoolPlayLin deleted the finance_zh_CN branch September 30, 2023 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewmayer matthewmayer matthewmayer approved these changes

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

@import-brain import-brain import-brain approved these changes

Assignees
No one assigned
Labels
c: locale Permutes locale definitions m: finance Something is referring to the finance module p: 1-normal Nothing urgent
Projects
None yet
Milestone
vAnytime
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@CoolPlayLin @ST-DDT @matthewmayer @import-brain @Shinigami92