Skip to content

do not change speed of playing decks if "Reset Speed/Tempo" preference is checked#1002

Merged
daschuer merged 3 commits intomixxxdj:masterfrom
Be-ing:unexpected_tempo_change_fix
Sep 6, 2016
Merged

do not change speed of playing decks if "Reset Speed/Tempo" preference is checked#1002
daschuer merged 3 commits intomixxxdj:masterfrom
Be-ing:unexpected_tempo_change_fix

Conversation

@Be-ing
Copy link
Copy Markdown
Contributor

@Be-ing Be-ing commented Sep 4, 2016

Fixes https://bugs.launchpad.net/mixxx/+bug/1603591

I'll write a test to prevent a regression, so don't merge this yet.

@Be-ing Be-ing force-pushed the unexpected_tempo_change_fix branch from 04fdb66 to 7da3e1f Compare September 4, 2016 23:43
@Be-ing Be-ing force-pushed the unexpected_tempo_change_fix branch from 7da3e1f to 9141ee1 Compare September 4, 2016 23:44
requires a little refactoring of MockedEngineBackendTest
@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Sep 5, 2016

Okay, test written and ready for review. I had to do some refactoring of MockedEngineBackendTest to do so. All the EngineSyncTest tests are still working. I can't confirm the refactoring didn't break any other tests because other tests are crashing on my system (on the master branch):
https://bugs.launchpad.net/mixxx/+bug/1620322
https://bugs.launchpad.net/mixxx/+bug/1620372

EDIT: Well, it looks like Travis is still passing all tests.

Comment thread src/engine/sync/enginesync.cpp Outdated

bool EngineSync::otherSyncedPlaying(const QString& group) {
int othersInSync = 0;
foreach (Syncable* theSyncable, m_syncables) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

use the new C++11 range based loop
for (Syncable* theSyncable: m_syncables)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@daschuer
Copy link
Copy Markdown
Member

daschuer commented Sep 6, 2016

Travis fails, not your fault:
The job exceeded the maximum time limit for jobs, and has been terminated.

@daschuer
Copy link
Copy Markdown
Member

daschuer commented Sep 6, 2016

LGTM Thank you!

@daschuer daschuer merged commit 107df69 into mixxxdj:master Sep 6, 2016
@Be-ing Be-ing deleted the unexpected_tempo_change_fix branch February 1, 2017 01:23
@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Sep 16, 2018

Good catch. Yes, I'll take a closer look at this.

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