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

fix setting of effect parameters programatically #4471

Merged
merged 1 commit into from
Oct 24, 2021
Merged

fix setting of effect parameters programatically #4471

merged 1 commit into from
Oct 24, 2021

Conversation

Be-ing
Copy link
Contributor

@Be-ing Be-ing commented Oct 22, 2021

@github-actions github-actions bot added the ui label Oct 22, 2021
@Be-ing Be-ing mentioned this pull request Oct 23, 2021
Merged
@Be-ing
Copy link
Contributor Author

Be-ing commented Oct 23, 2021 •
edited
Loading

ping

@uklotzde
Copy link
Contributor

Any insights about the cause and why this does not affect 2.3/main? Do we need comments in the code to prevent a "clever" refactoring in the future that could break it again?

@Be-ing
Copy link
Contributor Author

Be-ing commented Oct 23, 2021 •
edited
Loading

Don't know, don't care to investigate non-problems in obsolete code.

@Swiftb0y
Copy link
Member

Those who do not learn history are doomed to repeat it.

Even though the issue is obsolete, we should still document how this could break in the future.

@uklotzde
Copy link
Contributor

@daschuer Ping

daschuer
daschuer approved these changes Oct 24, 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 can confirm that the issue is fixed after merging this PR
LGTM, thank you for taking care.

@daschuer daschuer merged commit 30bdf83 into mixxxdj:effects_refactoring Oct 24, 2021
@Be-ing Be-ing deleted the main_eq branch October 24, 2021 21:51
@mixxxbot mixxxbot mentioned this pull request Aug 23, 2022
Closed
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
ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Be-ing @uklotzde @Swiftb0y @daschuer