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

Beats: Remove unnecessary capabilities #4103

Merged
merged 1 commit into from
Jul 17, 2021

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Jul 12, 2021

We currently have two beats implementations (BeatGrid and BeatMap)
and both support scaling and translating. Hence, it's unnecessary to
maintain these flags. In fact, the BEATSCAP_SCALE capability wasn't
even checked in the calling code but nobody noticed because both support
it anyway.

The BEATSCAP_ADDREMOVE and BEATSCAP_MOVEBEAT capabilities are only
supported by the BeatMap, but we don't have any code that makes use of
them. Therefore, these can be removed as well

We currently have two beats implementations (`BeatGrid` and `BeatMap`)
and both support scaling and translating. Hence, it's unnecessary to
maintain these flags. In fact, the `BEATSCAP_SCALE` capability wasn't
even checked in the calling code but nobody noticed because both support
it anyway.

The `BEATSCAP_ADDREMOVE` and `BEATSCAP_MOVEBEAT` capabilities are only
supported by the `BeatMap`, but we don't have any code that makes use of
them. Therefore, these can be removed as well.
@Holzhaus Holzhaus force-pushed the remove-beatscap-scale-translate branch from baa5fbd to 6a95caa Compare July 12, 2021 21:46
@coveralls
Copy link

coveralls commented Jul 12, 2021

Pull Request Test Coverage Report for Build 1024416249

  • 2 of 6 (33.33%) changed or added relevant lines in 3 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.0006%) to 28.643%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/track/beatmap.h 0 1 0.0%
src/engine/controls/bpmcontrol.cpp 1 4 25.0%
Files with Coverage Reduction New Missed Lines %
src/engine/readaheadmanager.cpp 2 88.79%
Totals Coverage Status
Change from base Build 1023749434: -0.0006%
Covered Lines: 20098
Relevant Lines: 70168

💛 - Coveralls

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.

YAGNI

@uklotzde uklotzde requested a review from daschuer July 12, 2021 23:19
@Holzhaus
Copy link
Member Author

Merge?

@Be-ing Be-ing merged commit 705955a into mixxxdj:main Jul 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants