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

Replace trackLoaded and trackUnloaded with a common Track changed signal #4059

Merged
merged 1 commit into from Jul 5, 2021

Conversation

daschuer
Copy link
Member

@daschuer daschuer commented Jul 5, 2021

This continues #4041 with the suggested common signal.

The pointed out issue with BpmControl is non, because it listens to the caching reader signals directly.
As far as I have tested there was also no issue with the original code.

However this version is less error prone and should save a few CPU cycles.

@uklotzde uklotzde added this to the 2.3.1 milestone Jul 5, 2021
@uklotzde
Copy link
Contributor

uklotzde commented Jul 5, 2021

Less than expected. I didn't notice that we have various trackLoaded/trackUnloaded signals.

Copy link
Contributor

@uklotzde uklotzde 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! LGTM

@uklotzde uklotzde merged commit 57670a4 into mixxxdj:2.3 Jul 5, 2021
@Holzhaus
Copy link
Member

Why did we merge this to 2.3? It sounds like this fixes not user-facing bug? Shouldn't have this went into main then?

@daschuer
Copy link
Member Author

#4041
Introduces a regression this is fixed here.
See the last comment in that PR.

@Holzhaus
Copy link
Member

Ah OK thanks. I was confused because this PR descriptions says:

As far as I have tested there was also no issue with the original code.

@daschuer daschuer deleted the lp1933991_2 branch August 1, 2021 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants