feat(music): add album 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 feat: add music.songName #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.

[matthewmayer]

1. We normally limit to 1000 entries per file
2. Suggest splitting into two PRs, one for adding data for existing methods (since that can be done in a patch release) and one for proposing new methods
3. Some of the genres look a bit awkward like "Holiday: Other". Maybe remove ones like that.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

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 😃

[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.

[xDivisionByZerox]

Some small updates to the documentation. Otherwise this looks good to me.

[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?

outdated