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

Traktor Kontrol S2MK3: Add documentation for PR#11702 #568

Merged
merged 2 commits into from
Jan 8, 2024
Merged

Traktor Kontrol S2MK3: Add documentation for PR#11702 #568

merged 2 commits into from
Jan 8, 2024

Conversation

olivier-mauras
Copy link

@olivier-mauras olivier-mauras mentioned this pull request Jul 3, 2023
Merged
@olivier-mauras
Copy link
Author

https://deploy-preview-568--mixxx-manual.netlify.app/hardware/controllers/native_instruments_traktor_kontrol_s2_mk3

ronso0
ronso0 requested changes Jul 3, 2023
View reviewed changes
Copy link
Member

@ronso0 ronso0 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, I've skimmed through it and left some comments

source/hardware/controllers/native_instruments_traktor_kontrol_s2_mk3.rst Outdated Show resolved Hide resolved
source/hardware/controllers/native_instruments_traktor_kontrol_s2_mk3.rst Outdated Show resolved Hide resolved
@olivier-mauras olivier-mauras force-pushed the update/ni-s2mk3 branch 2 times, most recently from 570cec4 to 00f8edd Compare July 4, 2023 05:33
@olivier-mauras
Copy link
Author

https://deploy-preview-568--mixxx-manual.netlify.app/hardware/controllers/native_instruments_traktor_kontrol_s2_mk3.html

@olivier-mauras olivier-mauras force-pushed the update/ni-s2mk3 branch 2 times, most recently from fdcbf81 to 6131dd9 Compare July 4, 2023 17:55
@ronso0
Copy link
Member

ronso0 commented Jul 5, 2023

Thanks.
Thing is, because you squashed the numbering change and the column width change and the actual Fx documentation commit... it's cumbersome to review the file diff. I didn't spot a regression but still it's not recommended to mix up separate change sets.

I'll take final look when the mapping PR gets a green light.

@JoergAtGithub
Copy link
Member

@Ronso Sorry I mixed the PRs for S2 Mk1 and Mk3 - the mapping PR is not ready yet

ronso0
ronso0 reviewed Dec 10, 2023
View reviewed changes
source/hardware/controllers/native_instruments_traktor_kontrol_s2_mk3.rst Outdated
Comment on lines 93 to 94
| Samples mode | If track is loaded into corresponding slot, go to CUE point and play | If track is playing, CUE default behaviour. |
| | | Otherwise eject track |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does pressing the button load the selected track?
Just wondering, because Shift+press can eject, and if that can be done why not also load.

ronso0
ronso0 approved these changes Dec 10, 2023
View reviewed changes
Copy link
Member

@ronso0 ronso0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Just one question about a part that was not touched by mixxxdj/mixxx#11702 IIUC, maybe just the documentation was missing.

@JoergAtGithub
Copy link
Member

LGTM! Thank you!

@JoergAtGithub JoergAtGithub merged commit 6609b2b into mixxxdj:2.4 Jan 8, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ronso0 ronso0 ronso0 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@olivier-mauras @ronso0 @JoergAtGithub