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.

Sign up for GitHub

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

Jump to bottom

CueControl: Use mixxx::audio::FramePos internally #4081

Merged
merged 4 commits into from
Jul 11, 2021
Merged

CueControl: Use mixxx::audio::FramePos internally #4081

merged 4 commits into from
Jul 11, 2021

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Jul 7, 2021

No description provided.

@Holzhaus Holzhaus mentioned this pull request Jul 8, 2021
Merged
10 tasks
uklotzde
uklotzde suggested changes Jul 9, 2021
View reviewed changes
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.

That was heavy. Found only one leftover debugging statement.

@daschuer Please double check

src/engine/controls/cuecontrol.cpp Outdated Show resolved Hide resolved
src/engine/controls/cuecontrol.cpp Outdated Show resolved Hide resolved
@Holzhaus
Copy link
Member Author

Holzhaus commented Jul 9, 2021

That was heavy.

Yeah sorry. Maybe I should have migrated the individual methods in separate commits/PRs.

@daschuer
Copy link
Member

daschuer commented Jul 9, 2021

LGTM.
I think that deserves also a manual tests.
Did you give it a try already? I can test tonight.

@uklotzde
Copy link
Contributor

@daschuer Ping. This is the last pending PR before the final cleanup.

daschuer
daschuer approved these changes Jul 11, 2021
View reviewed changes
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

I did a brief test and everything still works. Thank you.

@daschuer daschuer merged commit 0979e8e into mixxxdj:main Jul 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uklotzde uklotzde uklotzde approved these changes

@daschuer daschuer daschuer approved these changes

Assignees
No one assigned
Labels
code quality
Projects
Semanting Type Refactoring
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Holzhaus @daschuer @uklotzde