Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MBS-13369 (1/3): Also allow Bugs! MV links on releases #3098

Merged
merged 1 commit into from
Nov 20, 2023
Merged

MBS-13369 (1/3): Also allow Bugs! MV links on releases #3098

merged 1 commit into from
Nov 20, 2023

Conversation

Maxr1998
Copy link
Sponsor Contributor

@Maxr1998 Maxr1998 commented Nov 18, 2023 •
edited by yvanzo

Problem MBS-13369 (1/3)

Currently, music videos from Bugs! are only allowed on recordings.

Solution

Like YouTube videos, it would make sense to be able to add them to releases as well.

@Maxr1998 Maxr1998 changed the title Also allow Bugs! MV links on releases MBS-13369: Also allow Bugs! MV links on releases Nov 18, 2023
@yvanzo yvanzo changed the title MBS-13369: Also allow Bugs! MV links on releases MBS-13369 (1/3): Also allow Bugs! MV links on releases Nov 20, 2023
yvanzo
yvanzo approved these changes Nov 20, 2023
View reviewed changes
Copy link
Contributor

@yvanzo yvanzo left a comment

Choose a reason for hiding this comment

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

Thank you @Maxr1998 for submitting these three patches #3098, #3099, and #3100.

It looks good to me - code wise - but I did not test given that you provided tests with a good coverage.

However I would like @reosarevok to review the STYLE side of these three patches.

Notes for future submissions:

  • I’ve inserted parenthesis (x/3) in the title of your pull requests, as recommended in our contributing guidelines (at title), to clarify that each pull request is partially addressing the issue. Actually, those could have been 3 commits in the same pull request.
  • I’ve inserted the ticket’s reference in the description of your pull requests, as recommended in our contributing guidelines (at Comment) too, to make access to the ticket faster for the next reviewer.

This was referenced Nov 20, 2023
Merged
Merged
@Maxr1998
Copy link
Sponsor Contributor Author

Thanks. I created the three pull requests before making the Jira ticket, I shortly contemplated making three tickets + an epic to group them together but found that excessive. The PRs aren't exactly alike (Bugs! already had MV support for recordings), that's why I initially decided to split them up. Instead, creating a single PR with three commits would have likely been the way to go.

Thanks for the suggestions (and changes) for the PR title & description, I'll keep those in mind for the future.

reosarevok
reosarevok approved these changes Nov 20, 2023
View reviewed changes
Copy link
Member

@reosarevok reosarevok left a comment

Choose a reason for hiding this comment

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

Seems fine to add these IMO.

@reosarevok reosarevok merged commit f601aa5 into metabrainz:master Nov 20, 2023
3 checks passed
@Maxr1998
Copy link
Sponsor Contributor Author

Thanks for the merge!

@Maxr1998 Maxr1998 deleted the bugs-mvs branch November 20, 2023 15:38
mwiencek added a commit that referenced this pull request Nov 20, 2023
@mwiencek
Merge branch 'master' into beta
1a70b6b 
* master:
  Update POT files using the production database
  Simplify RegEx
  RemoveUnreferencedRows: Silence warning for already-deleted rows (#3102)
  MBS-13300: Delete RG cover art if release is moved (#3103)
  Rewrite more artist pages for Genie
  MBS-13372: Unused URL with gid redirect cannot be deleted (#3104)
  Add support for Genie MV links (#3100)
  Add support for Melon MV links (#3099)
  Also allow Bugs! MV links on releases (#3098)
  MBS-13284: Don't require edit note for alias removals (#3047)
  MBS-13366: Also show medium data on Edit medium preview (#3095)
  Block unverified editors from adding notes at the add_note level (#2969)
  MBS-13310: Add new empty artist credits to unreferenced_row_log (#3105)
  MBS-13326: Add documentation bubble for the artist sort name field (#3081)
  Give more details about the usage of admin/psql
  Allow connecting to any database with SYSTEM credentials
  Avoid ln that clashes with l calls
  Add a tool that can execute any file via Webpack (#3089)
  MBS-13273: Update track credits with matching prefixes (#3082)
  Add basic Selenium test for seeding RG URLs
  Add ReleaseGroup::Create basic test
  MBS-9396: Properly parse seeded release group URLs
yvanzo added a commit that referenced this pull request Nov 27, 2023
@yvanzo
Merge branch 'beta' into production
5d39ca7 
* beta:
  Translated using Weblate (French)
  Translated using Weblate (Italian)
  Update translation files
  Translated using Weblate (Italian)
  Translated using Weblate (German)
  MBS-13373: Update the Deezer logo (#3110)
  Update POT files using the production database
  Also clean up jazzmusicarchives search URLs
  MBS-13376: Handle metalmusicarchives.com links
  Set -w (page width) on msgcat and msguniq (#3106)
  Update POT file from the previous commit
  Standardize /utility strings and add context
  Update translation files
  Update POT file from the previous commit
  Standardize /utility strings and add context
  MBS-11934: Add report for releases without CAA (#3084)
  Update POT files using the production database
  Simplify RegEx
  Translated using Weblate (Chinese (Simplified))
  Translated using Weblate (Spanish (Latin America))
  Translated using Weblate (Lithuanian)
  Translated using Weblate (Italian)
  Update translation files
  RemoveUnreferencedRows: Silence warning for already-deleted rows (#3102)
  MBS-13300: Delete RG cover art if release is moved (#3103)
  Rewrite more artist pages for Genie
  MBS-13372: Unused URL with gid redirect cannot be deleted (#3104)
  Add support for Genie MV links (#3100)
  Add support for Melon MV links (#3099)
  Also allow Bugs! MV links on releases (#3098)
  MBS-13284: Don't require edit note for alias removals (#3047)
  MBS-13366: Also show medium data on Edit medium preview (#3095)
  Block unverified editors from adding notes at the add_note level (#2969)
  MBS-13310: Add new empty artist credits to unreferenced_row_log (#3105)
  MBS-13326: Add documentation bubble for the artist sort name field (#3081)
  Give more details about the usage of admin/psql
  Allow connecting to any database with SYSTEM credentials
  Avoid ln that clashes with l calls
  Add a tool that can execute any file via Webpack (#3089)
  MBS-13273: Update track credits with matching prefixes (#3082)
  Update POT files using the production database
  MBS-13362: Handle Spotify prerelease URLs (#3088)
  Fix formatting OAuth URI error messages (#3087)
  Use the same error message for all URL fields (#3086)
  MBS-13357: Clarify the report for low data quality (#3085)
  Translated using Weblate (German)
  RebuildIndexesUsingCollations.pl improvements (#3080)
  Add basic Selenium test for seeding RG URLs
  Add ReleaseGroup::Create basic test
  MBS-9396: Properly parse seeded release group URLs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reosarevok reosarevok reosarevok approved these changes

@yvanzo yvanzo yvanzo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@Maxr1998 @reosarevok @yvanzo