Skip to content

Conversation

@darrell-k
Copy link
Contributor

Do not cache the isClassique flag because it may need to be reevaluated due to user preference changes (Enhanced classical music display and Additional classical genres).

I didn't want to invalidate the entire Qobuz cache due to preference changes, so I've added a function to evaluate it every time it is needed.

@SamInPgh , please some regression testing (classical and non-classical albums, plugin browsing/play queue and import). Thanks.

@SamInPgh
Copy link
Contributor

@darrell-k I'm going to depend on your regression testing for this change, as I have neither the knowledge nor the classical content in my library to do a good job of it. I think I understand the logic and it seems fairly straightforward. Go ahead and merge it whenever you are ready.

@darrell-k
Copy link
Contributor Author

Although unlikely (scanning the code shows little usage of isClassique), could you just check it hasn't messed up non classical listings/playing/import? I always think an independent test is safest.

@SamInPgh
Copy link
Contributor

Okay. Before I do that, would it cause any problems if I created and merged a PR with a change to the trackGain() subroutine in ProtocolHandler.pm that I have been testing? I see that your PR also has changes to that module and I don't want to mess you up.

@darrell-k
Copy link
Contributor Author

Unlikely, let's try. Should be easy to sort out even if it does create a conflict.

@SamInPgh
Copy link
Contributor

Okay. I'll promote it later today (probably tomorrow for you) and merge it after code review.

@michaelherger
Copy link
Member

Okay. Before I do that, would it cause any problems if I created and merged a PR with a change to the trackGain() subroutine in ProtocolHandler.pm that I have been testing? I see that your PR also has changes to that module and I don't want to mess you up.

That's really what Git is all about: develop on different branches, bring them together in a controlled way. Yes, sometimes conflicts happen. But usually they can be resolved easily if it's only smallish changes in a file or two. Give it a try! If merging fails, it'll be a learning session 😉.

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.

3 participants