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

mixxx_controls: added track color controls #638

Open
wants to merge 3 commits into
base: 2.5
Choose a base branch
from

Conversation

aqw42
Copy link

@aqw42 aqw42 commented Apr 11, 2024

I have a few questions and will need a comprehensive review

Do i need to add the sampler control target ?
I don't think color or stars can apply to sample but correct me if i'm wrong

Is 2.4 the default branch or is it main ?

added :
[ChannelN]
[PreviewDeckN]
[SamplerN]
track_color_next
track_color_prev
track_color_selector

[Library]
track_color_next
track_color_prev
track_color_selector

added :
[ChannelN]
[PreviewDeckN]
stars_up
stars_down
track_color_next
track_color_prev
track_color_selector

[Library]
track_color_selector
@aqw42 aqw42 changed the title mixxx_controls: added track color and star control mixxx_controls: added track color controls Apr 11, 2024
@ronso0
Copy link
Member

ronso0 commented Apr 12, 2024

Do i need to add the sampler control target ?

Yes, stars controls are added for all decks (it's added here and addDeckAndPreviewDeckControl() adds the control for all players)

Same for the (deck) color controls.

@ronso0
Copy link
Member

ronso0 commented Apr 18, 2024

Is 2.4 the default branch or is it main ?

Good you asked (I'd almost have merged this to main):
Current stable is the default branch for fixups, so stars and next/prev color should go to 2.4
color selector though is a new feature that was merged to main, so that should go to 2.5, i.e. you need to revert the selector and move that to another branch targeting 2.5

@aqw42
Copy link
Author

aqw42 commented Jun 6, 2024

Is 2.4 the default branch or is it main ?

Good you asked (I'd almost have merged this to main): Current stable is the default branch for fixups, so stars and next/prev color should go to 2.4 color selector though is a new feature that was merged to main, so that should go to 2.5, i.e. you need to revert the selector and move that to another branch targeting 2.5

Time passed, I was pretty busy
Just to make sure nothing changed,
Should I create :

  • A new pull request with both the changes targeting 2.5 ?
  • One pull request with next/prev targeting 2.4, and another with selector targeting 2.5 ?

@ronso0
Copy link
Member

ronso0 commented Jun 6, 2024

Sorry this got little attention lately, there were PRs with higher prio for 2.5

This is a new feature and should therefore actually go to main. However, no official 2.5-beta has been created and we may argue whether this is an entirely new feature (main) or rather an extension of the [Library],track_color_.. controls (might slip into 2.5 last minute). If we decide for 2.5 we should merge before 2.5-beta becasue beta means string freeze (no new translatable strings added to source).

Therefore I suggest you rebase onto 2.5 for now, and if it doesn't make into 2.5 rebasing onto main later on will be easy.

@mixxxdj/developers What do you think? 2.5 or main?

@ronso0
Copy link
Member

ronso0 commented Jun 6, 2024

oh, sorry, I confused with the controls PR mixxxdj/mixxx#13077

@aqw42 aqw42 changed the base branch from 2.4 to 2.5 June 6, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants