feat(music): add albumName and artist methods

[jeremyhofer]

This PR introduces two new methods to the music module. albumName and artist.

Data was sourced from various resources from a number of resources online, including:

• The dataset from when the songName method was added in https://github.com/faker-js/faker/pull/996: https://manghammath.com/80s%20Hits/The%20Top%201000%20Songs%20of%20AllTime.pdf
• Rolling Stone 500 Greatest Albums: https://www.rollingstone.com/music/music-lists/best-albums-of-all-time-1062063/arcade-fire-%ef%bb%bffuneral-1062733/
• Chart Masters compiled data for Artists and Albums: https://chartmasters.org/

The @since version on the methods was set to 8.5.0, as it looks like the 8.4.0 tag has been created.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.95%. Comparing base (91a4d3d) to head (65c11e9).

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #2620      +/-   ##
==========================================
- Coverage   99.96%   99.95%   -0.01%     
==========================================
  Files        2973     2975       +2     
  Lines      212501   214451    +1950     
  Branches      948      601     -347     
==========================================
+ Hits       212422   214358    +1936     
- Misses         79       93      +14     

Files	Coverage Δ
src/locales/en/music/album.ts	100.00% <100.00%> (ø)
src/locales/en/music/artist.ts	100.00% <100.00%> (ø)
src/locales/en/music/index.ts	100.00% <100.00%> (ø)
src/modules/music/index.ts	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

[jeremyhofer]

@matthewmayer thanks for the feedback!

I pulled the genre and song name additions out of this PR. Will raise a PR to add and clean up the additional genres. Given the song name set is near 1000 entries I'm discarding all my changes there.

I updated the added album_name and artist sets down to 1000 entries each.

[matthewmayer]

Non ascii names are OK as long as that's how they are normally written in English

[matthewmayer]

FWIW although we would normally wait for 10 upvotes to approve a change like this this does seem a fairly sensible extension of the current music module so I'd be fine with approving this for next minor or major release.

[jeremyhofer]

Updated to remove albums that could be confusing - the ones that were called out, as well as others I felt could similarly cause some confusion. Updated to case insensitive sort the data as well.

Thanks for considering these methods! Would it be possible to add the ask to propose and vote on new modules and methods to the docs and CONTRIBUTING.md? I was unaware this was the general expectation when I got started in the repo :smiley:

[matthewmayer]

Would it be possible to add the ask to propose and vote on new modules and methods to the docs and CONTRIBUTING.md? I was unaware this was the general expectation when I got started in the repo 😃

Its not really a hard requirement, its more to help triage the issues that are added to the project. So if someone adds an issue saying "it would be cool if we had artist names" we will normally wait until >10 upvotes to see if other people would find it useful. If someone sources suitable data, makes a PR, adds tests etc, then the threshold for inclusion is lower!

So I think being too strict about it and documenting in CONTRIBUTING might discourage people from actually contributing code, which we wouldnt want.

[ST-DDT]

We talked about this PR in today's team meeting. There we discussed whether the methods (and definitions) should use the Name suffix. In our oppinion album and albumName convey the same information and thus we should only use album().

The definitions should follow the method's name unless there is a need for disambiaguation such as with _pattern. (Since we don't plan to add patterns for these methods we should use the method name for the definitions)

We will rename songName -> song in a separate PR.

@matthewmayer What do you think?

[matthewmayer]

Sure. Shorter is better like city instead of cityName

[ST-DDT]

@jeremyhofer Could you please apply the requested changes and fix the merge conflicts?

[netlify[bot]]

✅ Deploy Preview for fakerjs ready!

Name	Link
🔨 Latest commit	65c11e9675d59f935356549d5ce8db4ad78ef4d5
🔍 Latest deploy log	https://app.netlify.com/sites/fakerjs/deploys/662f626d57e84e000893c36b
😎 Deploy Preview	https://deploy-preview-2620.fakerjs.dev
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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