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

Use rate_ratio for track timing display #10916

Merged
merged 1 commit into from
Oct 9, 2022
Merged

Conversation

ywwg
Copy link
Member

@ywwg ywwg commented Sep 25, 2022

This prevents wild numbers from appearing during scratching under vinyl control.

(Without this, the track time can appear as extremely long or extremely short. The vinyl control code manipulates the rate_ratio value so that it stays within reasonable boundaries).

@ywwg ywwg requested a review from daschuer September 25, 2022 22:37
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.

LGTM
I think this is a bug-fix and can go to 2.3
can you rebase it?

@ywwg
Copy link
Member Author

ywwg commented Sep 26, 2022

can do

@ywwg ywwg changed the base branch from main to 2.3 September 26, 2022 16:00
@daschuer
Copy link
Member

There is a pre-commit issue the other CI failures are hopfully solved with #10904

@ywwg
Copy link
Member Author

ywwg commented Oct 2, 2022

this all good?

@daschuer
Copy link
Member

daschuer commented Oct 2, 2022

There is still a code style issue. Can you fix it?
If you rebase it again on 2.3 also the Ubuntu build failure will disappear

@ywwg ywwg force-pushed the temposecondsfix branch from 9622a65 to 3d3661a Compare October 4, 2022 19:25
@ywwg
Copy link
Member Author

ywwg commented Oct 4, 2022

oh I see. fixed

@ywwg
Copy link
Member Author

ywwg commented Oct 4, 2022

I fixed the style issue and now it wants to put it back the way it was before?

@daschuer
Copy link
Member

daschuer commented Oct 5, 2022

I the original version, there was a leading /
Now it is a long line issue. That is the disadvantage if our two stage clang-format call.

Do you have pre-commit installed, I wonder why this has slipped through.

@ywwg
Copy link
Member Author

ywwg commented Oct 8, 2022

I the original version, there was a leading / Now it is a long line issue. That is the disadvantage if our two stage clang-format call.

Do you have pre-commit installed, I wonder why this has slipped through.

precommit is broken on my machine and I have been unable to fix it:


more info:

virtualenv python version did not match created version:
- actual version: <<error retrieving version from /home/owen/.cache/pre-commit/repo2_q5mxbt/py_env-python3/bin/python>>
- expected version: 3.10.6.final.0

This prevents wild numbers from appearing during scratching under vinyl control.
@ywwg ywwg force-pushed the temposecondsfix branch from e3b4924 to d051deb Compare October 8, 2022 16:11
@ywwg
Copy link
Member Author

ywwg commented Oct 8, 2022

@ywwg
Copy link
Member Author

ywwg commented Oct 8, 2022

@daschuer looks good finally!

@daschuer daschuer added the changelog This PR should be included in the changelog label Oct 9, 2022
@daschuer daschuer added this to the 2.3.4 milestone Oct 9, 2022
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.

Thank you

@daschuer daschuer merged commit a2654a1 into mixxxdj:2.3 Oct 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This PR should be included in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants