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

BpmControl: Replace double sample positions with mixxx::audio::FramePos #4191

Merged
merged 2 commits into from
Aug 19, 2021
Merged

BpmControl: Replace double sample positions with mixxx::audio::FramePos #4191

merged 2 commits into from
Aug 19, 2021

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Aug 8, 2021

Based on #4189.

@coveralls
Copy link

coveralls commented Aug 8, 2021 •
edited
Loading

Pull Request Test Coverage Report for Build 1139897155

  • 114 of 167 (68.26%) changed or added relevant lines in 3 files are covered.
  • 128 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.003%) to 25.963%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/engine/enginebuffer.cpp 9 10 90.0%
src/engine/controls/bpmcontrol.cpp 104 156 66.67%
Files with Coverage Reduction New Missed Lines %
src/coreservices.cpp 128 0%
Totals Coverage Status
Change from base Build 1139376155: -0.003%
Covered Lines: 20000
Relevant Lines: 77034
💛 - Coveralls

@Holzhaus Holzhaus force-pushed the bpmcontrol-framepos branch 2 times, most recently from 2ca93e0 to 3d966c6 Compare August 16, 2021 23:27
@Holzhaus Holzhaus marked this pull request as ready for review August 16, 2021 23:28
@Holzhaus
Copy link
Member Author

I rebased on latest main, ready to review.

@Holzhaus Holzhaus mentioned this pull request Aug 17, 2021
Merged
uklotzde
uklotzde suggested changes Aug 17, 2021
View reviewed changes
src/engine/controls/bpmcontrol.cpp Outdated Show resolved Hide resolved
@Holzhaus Holzhaus requested a review from uklotzde August 17, 2021 16:27
uklotzde
uklotzde approved these changes Aug 19, 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.

Thank you! LGTM

@Holzhaus
Copy link
Member Author

Merge?

@uklotzde uklotzde merged commit 0774a04 into mixxxdj:main Aug 19, 2021
ronso0 pushed a commit to ronso0/mixxx that referenced this pull request Sep 24, 2021
@Holzhaus @ronso0
BpmControl: Add check to verify that loop positions are valid
515eb54 
As suggested here: mixxxdj#4191 (comment)
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

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 @coveralls @uklotzde