Skip to content

refactor(locale): group name entries by gender#2938

Merged
ST-DDT merged 19 commits into
nextfrom
refactor/locale/1677-group-name-entries-by-gender
Jun 30, 2024
Merged

refactor(locale): group name entries by gender#2938
ST-DDT merged 19 commits into
nextfrom
refactor/locale/1677-group-name-entries-by-gender

Conversation

@xDivisionByZerox

@xDivisionByZerox xDivisionByZerox commented Jun 8, 2024

Copy link
Copy Markdown
Member

Description

Groups locale name entries in the PersonModule by gender.

Additional Info

Fixes #1677.

Script that was used to refactor locale files automatically

ℹ️ Note
The file being .ts is important, so you are able to directly import() other .ts files.

import { execSync } from 'node:child_process';
import { readdir, stat, unlink, writeFile } from 'node:fs/promises';
import { join, resolve } from 'node:path';

const localeRootDir = resolve('./src/locales');
const localeDirNames = await readdir(localeRootDir);

async function pathExist(path: string) {
  return stat(path)
    .then(() => true)
    .catch(() => false);
}

for (const localeDirName of localeDirNames) {
  const personPath = join(localeRootDir, localeDirName, 'person');
  if (!(await pathExist(personPath))) {
    continue;
  }

  const getEntryPath = (entryName: string) =>
    join(personPath, `${entryName}.ts`);

  const entryGroups = [
    ['prefix', 'female_prefix', 'male_prefix'],
    ['first_name', 'female_first_name', 'male_first_name'],
    ['middle_name', 'female_middle_name', 'male_middle_name'],
    ['last_name', 'female_last_name', 'male_last_name'],
    ['last_name_pattern', 'female_last_name_pattern', 'male_last_name_pattern'],
  ];
  for (const entryGroup of entryGroups) {
    const [mainEntry, femaleEntry, maleEntry] = entryGroup;
    const mainEntryPath = getEntryPath(mainEntry);
    const femaleEntryPath = getEntryPath(femaleEntry);
    const maleEntryPath = getEntryPath(maleEntry);

    const finalMainEntry: Record<string, unknown> = {};

    const keyPathMapList = [
      { key: 'generic', path: mainEntryPath },
      { key: 'female', path: femaleEntryPath },
      { key: 'male', path: maleEntryPath },
    ];
    for (const { key, path } of keyPathMapList) {
      if (!(await pathExist(path))) {
        continue;
      }

      const { default: defaultExportMember } = await import(`file://${path}`);
      if (!defaultExportMember) {
        continue;
      }

      finalMainEntry[key] = defaultExportMember;
    }

    // remove in second cycle in case files import each other
    for (const { path } of keyPathMapList) {
      await unlink(path).catch((): void => undefined);
    }

    if (Object.keys(finalMainEntry).length === 0) {
      continue;
    }

    await writeFile(
      mainEntryPath,
      `export default ${JSON.stringify(finalMainEntry)}`
    );
    execSync(`npx prettier --write ${mainEntryPath}`);
  }
}

Note

The script did not account for non-applicable values. These have been manually justified in 4b86dcb. A list of affected files can be found here.

@xDivisionByZerox xDivisionByZerox added p: 1-normal Nothing urgent c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs c: locale Permutes locale definitions breaking change Cannot be merged when next version is not a major release m: person Something is referring to the person module labels Jun 8, 2024
@xDivisionByZerox xDivisionByZerox added this to the v9.0 milestone Jun 8, 2024
@xDivisionByZerox xDivisionByZerox self-assigned this Jun 8, 2024
@netlify

netlify Bot commented Jun 8, 2024

Copy link
Copy Markdown

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit 3e2bb1b
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/667f1694a52312000882d4fe
😎 Deploy Preview https://deploy-preview-2938.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.

@codecov

codecov Bot commented Jun 9, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.95%. Comparing base (92a2f17) to head (3e2bb1b).

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #2938      +/-   ##
==========================================
- Coverage   99.95%   99.95%   -0.01%     
==========================================
  Files        2984     2748     -236     
  Lines      216057   239091   +23034     
  Branches      597      586      -11     
==========================================
+ Hits       215969   238983   +23014     
- Misses         88      108      +20     
Files Coverage Δ
src/index.ts 100.00% <100.00%> (ø)
src/locales/af_ZA/person/first_name.ts 100.00% <100.00%> (ø)
src/locales/af_ZA/person/index.ts 100.00% <ø> (ø)
src/locales/af_ZA/person/last_name.ts 100.00% <100.00%> (ø)
src/locales/af_ZA/person/last_name_pattern.ts 100.00% <100.00%> (ø)
src/locales/ar/location/street_pattern.ts 100.00% <100.00%> (ø)
src/locales/ar/person/first_name.ts 100.00% <100.00%> (ø)
src/locales/ar/person/index.ts 100.00% <ø> (ø)
src/locales/ar/person/last_name.ts 100.00% <100.00%> (ø)
src/locales/ar/person/last_name_pattern.ts 100.00% <100.00%> (ø)
... and 174 more

... and 264 files with indirect coverage changes

Comment thread src/definitions/person.ts Outdated
@Shinigami92 Shinigami92 added the needs rebase There is a merge conflict label Jun 20, 2024
@xDivisionByZerox xDivisionByZerox removed the needs rebase There is a merge conflict label Jun 23, 2024
@xDivisionByZerox xDivisionByZerox marked this pull request as ready for review June 23, 2024 15:08
@xDivisionByZerox xDivisionByZerox requested a review from a team as a code owner June 23, 2024 15:08
@xDivisionByZerox xDivisionByZerox requested a review from a team June 23, 2024 15:08
Comment thread src/definitions/person.ts Outdated
Comment thread src/modules/person/index.ts Outdated
Comment thread src/modules/person/index.ts Outdated
Comment thread src/modules/person/index.ts Outdated
Comment thread src/modules/person/index.ts Outdated
Comment thread src/modules/person/index.ts Outdated
Comment thread src/modules/person/index.ts Outdated
Comment thread src/modules/person/index.ts Outdated
Comment thread src/modules/person/index.ts Outdated
Comment thread src/modules/person/index.ts
Comment thread src/modules/person/index.ts Outdated

@Shinigami92 Shinigami92 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I did not look into the 606 locales changes... The rest looks already really good to me, just some open questions about naming and wording

Comment thread src/modules/person/index.ts Outdated
Comment thread src/definitions/person.ts Outdated
@ST-DDT ST-DDT added the s: needs decision Needs team/maintainer decision label Jun 24, 2024
ST-DDT
ST-DDT previously approved these changes Jun 24, 2024

@ST-DDT ST-DDT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, only the following things need to be discussed:

  • Name of the helper type
  • Location of the type definition
  • Exact generic shape of the type
@ST-DDT ST-DDT added needs rebase There is a merge conflict labels Jun 26, 2024
@ST-DDT

ST-DDT commented Jun 26, 2024

Copy link
Copy Markdown
Member

Oh I'm sorry, looks like I caused a merged conflict 🙇‍♂️

@xDivisionByZerox

Copy link
Copy Markdown
Member Author

Oh I'm sorry, looks like I caused a merged conflict 🙇‍♂️

All good, happens.I resolved them to the best of my abilities.

@xDivisionByZerox xDivisionByZerox removed the needs rebase There is a merge conflict label Jun 27, 2024
@ST-DDT ST-DDT removed the s: needs decision Needs team/maintainer decision label Jun 27, 2024
Comment thread src/modules/person/index.ts Outdated
Comment thread src/modules/person/index.ts Outdated
@ST-DDT ST-DDT merged commit e21fcaf into next Jun 30, 2024
@ST-DDT ST-DDT deleted the refactor/locale/1677-group-name-entries-by-gender branch June 30, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Cannot be merged when next version is not a major release c: locale Permutes locale definitions c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs m: person Something is referring to the person module p: 1-normal Nothing urgent

3 participants