MBS-12552 (V): Refactor JS code for central entity #2732
Conversation
root/types/entity.js
Outdated
| @@ -66,6 +66,12 @@ declare type CoreEntityTypeT = | |||
| | NonUrlCoreEntityTypeT | |||
| | 'url'; | |||
|
|
|||
| declare type CreditableEntityT = | |||
There was a problem hiding this comment.
Creditable sounds like "it can be given a credit", which is a bit confusing because we already have entities that can get credits when used in relationships and others which can't. Is EntityWithArtistCreditsT very bad? :)
There was a problem hiding this comment.
No problem with that. Replaced with commit 7c97f77.
root/types/entity.js
Outdated
| @@ -7,6 +7,20 @@ | |||
| * later version: http://www.gnu.org/licenses/gpl-2.0.txt | |||
| */ | |||
|
|
|||
| declare type AliasableEntityT = | |||
There was a problem hiding this comment.
"Aliasable" seems super strange. I'd probably go with EntitiesWithAliasesT, honestly.
There was a problem hiding this comment.
Replaced with commit e5dd97e.
root/types/recording.js
Outdated
| @@ -10,13 +10,12 @@ | |||
| // MusicBrainz::Server::Entity::Recording::TO_JSON | |||
| declare type RecordingT = $ReadOnly<{ | |||
| ...AnnotationRoleT, | |||
| ...ArtistCreditRoleT, | |||
There was a problem hiding this comment.
The change here moves from "either has them or has not" to "certainly has them" for both artist and artistCredit. Is this intended?
There was a problem hiding this comment.
In fact, I see it's breaking flow, so probably not intended :)
There was a problem hiding this comment.
Indeed that wasn’t supposed to be pushed. Dropped it. I gave up refining Flow types at that point.
It follows commit 41a4b69 in PR metabrainz#2731.
|
For reference, the discussion about the term “Central” continued in comment to the ticket. The logic would be to review the pull request #2731 first since it is ready and it precedes this one (which is a draft) as mentioned in the top-level comment. (Restored the intentionally nested items of the checklist.) |
|
Rebased, amended and merged the 5 first commits unto the pull request #2885. We will advise the renaming to |
Problem MBS-12552
The term “core” has been used so far with different meanings (some entities, public domain data, search indexes).
Solution
Following the pulll request #2731, it replaces this term with "central" in JavaScript code.
Checklist for author
Action