Skip to content
This repository has been archived by the owner on Jun 25, 2021. It is now read-only.

Fixes grouping albums by album artist rather than just album in the Music app #1

Merged
merged 3 commits into from
Sep 13, 2019

Conversation

bhdouglass
Copy link
Member

No description provided.

@UniversalSuperBox
Copy link
Member

Is there an issue filed? Where would I test this?

@bhdouglass
Copy link
Member Author

Here is the MR for the music app: https://gitlab.com/ubports/apps/music-app/merge_requests/46
And here is the original issue: https://gitlab.com/ubports/apps/music-app/issues/44

If you build this with crossbuilder and compile the music app from that branch you should be able to test the changes (just make sure that you have a multi-artist album on your phone).

@bhdouglass
Copy link
Member Author

Removed my build fixes and updated to include the changes from xenial_-_mediascanner-build. 🤞 hopefully it builds now!

@Flohack74
Copy link
Member

Unfortunately it does not ;)

@bhdouglass
Copy link
Member Author

Dangit!

@bhdouglass
Copy link
Member Author

5th time, maybe?

@bhdouglass
Copy link
Member Author

yay!

@bhdouglass
Copy link
Member Author

@UniversalSuperBox is this good to go?

Copy link
Member

@UniversalSuperBox UniversalSuperBox left a comment

Choose a reason for hiding this comment

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

This tests fine to me, however the commits from #2 have been pulled in uncleanly. Please rebase your branch or reopen this PR to have a clean commit log again.

@bhdouglass
Copy link
Member Author

Ok, should be good now, provided tests pass again.

Copy link
Member

@UniversalSuperBox UniversalSuperBox left a comment

Choose a reason for hiding this comment

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

This makes backwards-incompatible API changes. If we must do this, we'll need to also determine which apps link against this and create a framework version that is not backwards compatible.

@bhdouglass
Copy link
Member Author

@UniversalSuperBox I updated the Album class to not have breaking changes. But the dbus stuff was less straight forward. But since the dbus should just be used by the included qml lib I think everything should be fine.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants