refactor(person)!: flatten jobs definitions

[matthewmayer]

trafficstars

Currently the definitions for jobArea, jobDescriptor and jobType methods are nested inside a title.ts file. This causes undesired errors (ref #2503 ) when some of the job definitions are supplied but not others (e.g. in fr, ur, ar).

This removes the special cases, and just makes job_area, job_descriptor and job_title simple string arrays like other definitions.

The bulk locale changes were done with a simple script followed by linting.

This changes definition files but we generally don't consider those breaking changes when only accessed via faker.helpers.fake? Could be deferred until 9.0 if considered breaking.

const { allLocales } = require('@faker-js/faker');
const fs = require('fs');
const keys = Object.keys(allLocales);
for (let key of keys) {
  if (!['base'].includes(key)) {
    const localFaker = allLocales[key];
    if (localFaker.person.title) {
      const title = localFaker.person.title;
      if (title.level) {
        write('job_area', title.level, key);
      }
      if (title.descriptor) {
        write('job_descriptor', title.descriptor, key);
      }
      if (title.job) {
        write('job_type', title.job, key);
      }
      try {
        fs.rmSync("src/locales/" + key + "/person/title.ts")
      } catch (e) {}
    }
  }
}

function write(method, definitions, locale) {
  console.log(method, locale, definitions.length);
  fs.writeFileSync(
    'src/locales/' + locale + '/person/' + method + '.ts',
    'export default ' + JSON.stringify(definitions, null, 2)
  );
}

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 99.65428% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 99.56%. Comparing base (682a427) to head (3943cc9).

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #2505      +/-   ##
==========================================
- Coverage   99.57%   99.56%   -0.01%     
==========================================
  Files        2815     2846      +31     
  Lines      249845   249892      +47     
  Branches     1064     1055       -9     
==========================================
+ Hits       248777   248815      +38     
+ Misses       1068     1048      -20     
- Partials        0       29      +29     

Files	Coverage Δ
src/index.ts	100.00% <ø> (ø)
src/locales/ar/person/index.ts	100.00% <100.00%> (ø)
src/locales/ar/person/job_type.ts	100.00% <100.00%> (ø)
src/locales/el/person/index.ts	100.00% <100.00%> (ø)
src/locales/el/person/job_area.ts	100.00% <100.00%> (ø)
src/locales/el/person/job_descriptor.ts	100.00% <100.00%> (ø)
src/locales/el/person/job_type.ts	100.00% <100.00%> (ø)
src/locales/en/person/index.ts	100.00% <100.00%> (ø)
src/locales/en/person/job_area.ts	100.00% <100.00%> (ø)
src/locales/en/person/job_descriptor.ts	100.00% <100.00%> (ø)
... and 59 more

... and 28 files with indirect coverage changes

[matthewmayer]

Keep in mind that the prebuild faker instances contain more locale data than the actual locale. You should itterate over the local objects instead in your generation script. This should also reduce the diff significantly (due to current false positives).

My bad I thought rawDefinitions was for that. Will fix for the locales that were previously inheriting.

[matthewmayer]

I noticed that the sk, cs_CZ, he, pl data are just untranslated copies of en. Would it make sense to remove them as part of this PR?

[ST-DDT]

I noticed that the sk, cs_CZ, he, pl data are just untranslated copies of en. Would it make sense to remove them as part of this PR?

If the files are just plain copies, we can remove them now and the users wouldnt get any breakages. IMO We dont have to wait for v9 with that.

[ST-DDT]

Please fix the "merge conflicts" (although I think it will most likely merge locally without issues).

[matthewmayer]

if this won't be merged until v9 is it worth bothering to fix merge conflicts until we are ready to start on v9?

[ST-DDT]

No, there is no need to fix them now. I just left a comment to go along with the needs rebase label for improved visibility.

[matthewmayer]

OK :D 90% of my notifications for this project are about merge commits, so i wont bother adding to the volume for now!

[ST-DDT]

OK :D 90% of my notifications for this project are about merge commits, so i wont bother adding to the volume for now!

I wished there was a way to turn them off (or get them merged 😉)...

[ST-DDT]

Ready for review

[ST-DDT]

3 approving reviews. Enabling auto-merge.