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

Stanton DJC.4 mapping #2607

Merged
merged 11 commits into from Apr 4, 2020
Merged

Stanton DJC.4 mapping #2607

merged 11 commits into from Apr 4, 2020

Conversation

nuess0r
Copy link
Contributor

@nuess0r nuess0r commented Mar 28, 2020

Mapping for the Stanton DJC.4 controller.

Documentation: https://mixxx.org/wiki/doku.php/stanton_djc.4

I tried to follow the contribute mapping guide and the suggestions made in Denon MC7000 pull request discussion.

The files pass the pre-commit hooks on my machine (clang disabled, because it's not installed).

res/controllers/Stanton-DJC-4-scripts.js Show resolved Hide resolved
res/controllers/Stanton-DJC-4-scripts.js Outdated Show resolved Hide resolved
res/controllers/Stanton-DJC-4-scripts.js Outdated Show resolved Hide resolved
res/controllers/Stanton-DJC-4-scripts.js Outdated Show resolved Hide resolved
@Swiftb0y
Copy link
Member

Thanks for using components JS. This makes reviewing much more straight forward. Mapping looks pretty good so far.

Copy link
Member

@Holzhaus Holzhaus 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 very much for your contribution. The code looks very good - it's cool that you used our components library. The documentation on the wiki is great and I'm sure other users that have this controller will benefit from that.

I found some extremely minor code style issues. I'll merge when they're resolved.

res/controllers/Stanton-DJC-4-scripts.js Outdated Show resolved Hide resolved
res/controllers/Stanton-DJC-4-scripts.js Outdated Show resolved Hide resolved
res/controllers/Stanton-DJC-4.midi.xml Outdated Show resolved Hide resolved
- Changed class name to uppercase (DJC4 instead of djc4)
- Improved controller description
@Holzhaus
Copy link
Member

Holzhaus commented Apr 4, 2020

Can you add a commit that adds an entry to CHANGELOG.md?

@Holzhaus Holzhaus changed the base branch from master to 2.2 April 4, 2020 10:19
@Holzhaus
Copy link
Member

Holzhaus commented Apr 4, 2020

Please run git pull upstream 2.2 and resolve the merge conflicts.

Copy link
Member

@Holzhaus Holzhaus 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 very much! LGTM.

@Holzhaus Holzhaus merged commit 009598f into mixxxdj:2.2 Apr 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants