-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
RateControl/Position ScratchController: use std::unique_ptr, PollingControlProxy etc. #14058
base: 2.5
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple nits, address as you wish.
src/engine/controls/ratecontrol.cpp
Outdated
m_pJog = std::make_unique<ControlObject>(ConfigKey(group, QStringLiteral("jog"))); | ||
m_pJogFilter = std::make_unique<Rotary>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a refactoring, right? Should we rebase on main
instead? I know I changed this line in main
in #13853 so there are bound to be conflicts...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a refactoring, right? Should we rebase on main instead?
Yes, and no: as metioned above, these are just preparational steps for the scratch fix #14059 which should go to 2.5 IMO
5112667
to
5b63d77
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise.
df3c7b8
to
9c033fa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did another round and found another couple nits. Pls address and rebase squash afterwards so we can merge.
…ntrolProxy etc. Co-authored-by: Swiftb0y <[email protected]>
9c033fa
to
65f0d9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you. Waiting for CI.
Just some preparation for #14059
Targeting 2.5 because #14059 (or an alternative) may become an appropriate bug fix.