Skip to content

Kontrol S4 Mk3 mapping#10937

Closed
rah2501 wants to merge 1 commit intomixxxdj:mainfrom
rah2501:kontrol_s4_mk3
Closed

Kontrol S4 Mk3 mapping#10937
rah2501 wants to merge 1 commit intomixxxdj:mainfrom
rah2501:kontrol_s4_mk3

Conversation

@rah2501
Copy link
Copy Markdown

@rah2501 rah2501 commented Oct 3, 2022

Submitting Be-ing's mapping for consideration.

Copy link
Copy Markdown
Contributor

@Be-ing Be-ing left a comment

Choose a reason for hiding this comment

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

This mapping is incomplete.

Do not ship it

@rah2501
Copy link
Copy Markdown
Author

rah2501 commented Oct 4, 2022

Do not ship it

You've misunderstood and misrepresented what that campaign is about. The campaign is against distributions shipping patches in packages which they've taken from version control systems and are not intended to be shipped by project developers. (I should know, I used to work on mobile Linux with a number of the project developers involved in the campaign.)

This Mixxx pull request contains code that the patch author considers should not be committed. It does not contain code that the project developers consider should not be committed, at least no project developers have voiced any objections as yet. This is a different situation to what the "Don't Ship It" campaign is campaigning against.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Oct 4, 2022

You are misrepresenting incomplete code as if it is ready to be widely distributed to users... while also doing nothing whatsoever to help with the code -- and not even being able to, because you don't have the hardware -- and flagrantly ignoring established procedures. This pull request is trolling at worst and irresponsible at best.

@rah2501
Copy link
Copy Markdown
Author

rah2501 commented Oct 4, 2022

You are misrepresenting incomplete code as if it is ready to be widely distributed to users

I'm not misrepresenting, I'm presenting incomplete code as if it is ready to be widely distributed to users. Code being incomplete is not, in and of itself, a reason not to distribute code to users.

Incomplete code can be and in many cases is, useful. Such as this case.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Oct 4, 2022

You're not even capable of making that judgement without having the hardware.

Copy link
Copy Markdown
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

This is simply just a blatant reupload of #4056.

While we usually appreciate when people offer to pick up PRs that have gone stale (such as the PR introducing this controller mapping), those contributors usually continue the remaining work. This is not the case here. If you look at the original PR, I left a bunch of review comments outlining a part (I only reviewed 1/4th of the code) of what still needs to be done. The current state of the mapping does not meet our standards for inclusion. Nor can we confirm if it actually works without a trusted party with access to the hardware. If you want to help us with getting this mapping merged, please look at my review comments (in the previous PR) and work on fixing those. To do that, you'll need the controller hardware for testing and debugging. Here are some resources to get you started on controller mapping:
https://github.com/mixxxdj/mixxx/wiki/Contributing%20mappings

Please note that just opening a PR with a working mapping is not a guarantee of timely (or even at all) inclusion. We have many PRs and too few people to review them.

@rah2501
Copy link
Copy Markdown
Author

rah2501 commented Oct 5, 2022

You're not even capable of making that judgement without having the hardware.

I don't need the hardware in order to read testimony from others (including yourself) who find the mapping useful.

Let me quote:

my mapping in #4056 is in a usable state

-- https://mixxx.zulipchat.com/#narrow/stream/113295-controller-mapping/topic/Traktor.20Kontrol.20S4.20MK3/near/251335771

@rah2501
Copy link
Copy Markdown
Author

rah2501 commented Oct 5, 2022

This is simply just a blatant reupload of #4056.

Indeed. I stated that explicitly in the description.

I left a bunch of review comments

I see. I hadn't realised there were outstanding review comments.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Oct 5, 2022

There's no point continuing here if you're just going to argue without even attempting to help.

@Be-ing Be-ing closed this Oct 5, 2022
@mixxxdj mixxxdj locked as off-topic and limited conversation to collaborators Oct 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants