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(finance): add ISO 4217 numerical codes to Currency object #3404

Open
wants to merge 19 commits into
base: next
Choose a base branch
from

Conversation

Nfloc
Copy link

@Nfloc Nfloc commented Feb 17, 2025

Address issue #2289

added numericCode parameter to currency object in all languages (put zero for all non ISO currency (cypto currency)).

added currencyNumericCode to finance definition.

added currencyNumericCode testing to finance.spec.ts

changed descriptions to match new parameter.

added random currencyNumericCode method.

added numID parameter to currency object in all languages (put zero for all non ISO currency (cypto currency)).

added currencyNumID to finance definition.

added currencyNumID testing to finance.spec.ts

changed descriptions to match new parameter.

added random currencyNumID method
@Nfloc Nfloc requested a review from a team as a code owner February 17, 2025 03:31
Copy link

netlify bot commented Feb 17, 2025

Deploy Preview for fakerjs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit facfe98
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/67b2ad8255827200083ff7a3
😎 Deploy Preview https://deploy-preview-3404.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

This comment was marked as duplicate.

Copy link

codecov bot commented Feb 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.97%. Comparing base (ec7c9a8) to head (8916dd9).

Additional details and impacted files
@@           Coverage Diff            @@
##             next    #3404    +/-   ##
========================================
  Coverage   99.97%   99.97%            
========================================
  Files        2811     2811            
  Lines      216972   217398   +426     
  Branches      943      941     -2     
========================================
+ Hits       216919   217345   +426     
  Misses         53       53            
Files with missing lines Coverage Δ
src/definitions/finance.ts 100.00% <ø> (ø)
src/locales/el/finance/currency.ts 100.00% <100.00%> (ø)
src/locales/en/finance/currency.ts 100.00% <100.00%> (ø)
src/locales/fa/finance/currency.ts 100.00% <100.00%> (ø)
src/locales/fr/finance/currency.ts 100.00% <100.00%> (ø)
src/locales/zh_CN/finance/currency.ts 100.00% <100.00%> (ø)
src/modules/finance/index.ts 100.00% <100.00%> (ø)

src/modules/finance/index.ts Outdated Show resolved Hide resolved
src/modules/finance/index.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT added c: feature Request for new feature p: 1-normal Nothing urgent m: finance Something is referring to the finance module labels Feb 17, 2025
@ST-DDT ST-DDT linked an issue Feb 17, 2025 that may be closed by this pull request
@ST-DDT ST-DDT added this to the vAnytime milestone Feb 17, 2025
src/locales/fa/finance/currency.ts Outdated Show resolved Hide resolved
src/modules/finance/index.ts Outdated Show resolved Hide resolved
Nfloc added 10 commits February 17, 2025 13:49
Changed previous parameter numID --> numericCode.
Changed comments to reflect change and newest release.
Changed parameter numID --> numericCode
Changed parameter numID --> numericCode
Changed parameter numID --> numericCode.
Removed comment dev artifacts.
Changed parameter numID --> numericCode.
Removed dev artifact comments
Changed parameter numID --> numericCode.
Removed dev artifact comments.
Changed comment from numID to numericCode
Updated to new numericCode naming (numID previous)
Changed parameter numID --> numericCode
Changed method currencyNumID --> currencyNumericCode
Changed method currencyNumID --> currencyNumericCode
src/modules/finance/index.ts Outdated Show resolved Hide resolved
src/modules/finance/index.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT changed the title feat(finance): Added ISO 4217 numerical codes to Currency object feat(finance): add ISO 4217 numerical codes to Currency object Feb 17, 2025
chore(deps): lock file maintenance (faker-js#3403)
finance object parameter numericCode has been updated to a string.
numericCode has been given significant zeros. (e.g. '8' -> '008')
fix: test before using Buffers (faker-js#3400)
@Nfloc
Copy link
Author

Nfloc commented Feb 19, 2025

Should I update my original pull request description to fit the new parameter names?

src/locales/fa/finance/currency.ts Outdated Show resolved Hide resolved
src/modules/finance/index.ts Outdated Show resolved Hide resolved
src/modules/finance/index.ts Outdated Show resolved Hide resolved
src/modules/finance/index.ts Outdated Show resolved Hide resolved
Nfloc and others added 4 commits February 19, 2025 22:27
add grave markers around numerical code description.

Co-authored-by: ST-DDT <[email protected]>
change numerical ID to numeric code in description.

Co-authored-by: ST-DDT <[email protected]>
change number ID to numeric code in description.

Co-authored-by: ST-DDT <[email protected]>
fa language file was overridden by zh_CHN langauge file, changes reverted back to original.
it('should return a string', () => {
const currencyNumericCode = faker.finance.currencyNumericCode();

expect(currencyNumericCode).toBeTypeOf('string');
Copy link
Contributor

Choose a reason for hiding this comment

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

also check its 3 characters long?

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 m: finance Something is referring to the finance module p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add numeric currency code by ISO 4217 standard
4 participants