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.

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

Album sorting by role #1322

Closed
wants to merge 10 commits into from

Conversation

darrell-k
Copy link
Contributor

Allow sorting by any role which exists for the set of albums being displayed.

This PR implements the changes necessary for Default Skin, and additional changes to allow this functionality also for Material Skin (PR in progress).

Submitted for LMS 9.1 (9.0.2/3 would be OK too, do we want to?)

@darrell-k
Copy link
Contributor Author

Missing DCO signoff on older commits here. I tried to rebase as per the github suggested resolution, but it's failing, probably because of the merge of public/9.1 into my branch that I did tonight. I could just get rid of this PR and my branch and reapply the changes in a single commit on a new branch of 9.1? What do you think?

@michaelherger
Copy link
Member

9.1 is ok. I already spent quite some time on the SQL... but it was probably too early in the day, I haven't fully grokked it yet... so this note is just to tell you that I am in the process of reviewing. But it'll take some time (I have less spare time when on vacation, when on workdays 😂).

@michaelherger
Copy link
Member

BTW: given the age of the branch, the "oops" commits etc. I'd probably start a new branch off of the head, and port over the changes. But it's ok.

@darrell-k
Copy link
Contributor Author

Replaced by #1323

@darrell-k darrell-k closed this Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants