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

added missing space to engine controller API documentation #14384

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

13dixi37
Copy link
Contributor

guidelines say make commits as small as possible, so here we are

disclaimer:
please bear with me for I have very little experience with coding and this is my very first time contributing on github ever <3

@ronso0
Copy link
Member

ronso0 commented Feb 20, 2025

I think every contributor "started small" but you definitely made a mark here 🤣

Here's the usual paperwork, certainly not for this PR though, but in eager anticipation of more to come ; )


Please sign the Mixxx Contributor Agreement and comment here when you have done so. It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future.

@ronso0 ronso0 changed the title added missing space added missing space to engine controller API documentation Feb 20, 2025
@13dixi37
Copy link
Contributor Author

13dixi37 commented Feb 20, 2025

I think every contributor "started small" but you definitely made a mark here 🤣

Doing my part 🫡

Please sign the Mixxx Contributor Agreement

done

@ronso0
Copy link
Member

ronso0 commented Feb 20, 2025

Alright, thanks!

What's that Herculse mapping merge all about?

@13dixi37
Copy link
Contributor Author

oh this was because of me not properly understanding how prs work. i just wanted to merge my rmx branch into my main. i had no idea that it would show up here

got this old controller recently and wanted to update the mapping. the one in main is for mixxx 1.1. my plan is to improve this really nice 2.1 one i found to turn this oldtimer into a usable controller that's 2.5 ready. it's insanely well manufactured, looks cool and is fairly cheap when bought used. some features just didn't age well; like you can see that this was an early attempt at pioneering (pun intended) into digital djing without really knowing yet what features and layouts make sense and which don't (main vol fader between the deck faders would be one example. try playing a proper set without even touching it. had to tape that thing haha)

@ronso0
Copy link
Member

ronso0 commented Feb 20, 2025

Ah, okay.
main is not a good name for a fix or feature branch because it can cause hazzle when maintainers do fixups or squash (and for the reason you just noticed ; )
(next time) please pick a descriptive branch name.
That info is kinda hidden in the wiki https://github.com/mixxxdj/mixxx/wiki/using-git#create-a-new-branch

@ronso0
Copy link
Member

ronso0 commented Feb 20, 2025

Please make sure this branch contains only f6dba8a
Either remove the Hercules commits and move your production branch.
Or create a new branch eg. add-space, cherry-pick f6dba8a, push, close this PR and opne a one with add-space.

@ronso0
Copy link
Member

ronso0 commented Feb 20, 2025

@acolombier No need to review the Hercules mapping, it slipped in here by accident (see convo above)
Thanks for the quick review though!

@acolombier
Copy link
Member

Ah I'm sorry for the noise here, jumped to quickly to the Files changed tab 😓

@13dixi37
Copy link
Contributor Author

thanks nonetheless :D

@ronso0
Copy link
Member

ronso0 commented Feb 21, 2025

@13dixi37 please remove the Hercules commits.
For example with
git rebase -i a0c04c980530540276c9bcd5a271102fa07b5bb8^1
Then remove all commits but the space commit
Then force-push to your main branch.

@13dixi37 13dixi37 marked this pull request as ready for review February 21, 2025 16:02
@13dixi37
Copy link
Contributor Author

done!

sorry for the hassle and thanks for the help <3

Copy link
Member

@acolombier acolombier left a comment

Choose a reason for hiding this comment

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

LGTM!

Welcome to Mixxx community :D

@acolombier acolombier merged commit 0a92945 into mixxxdj:main Feb 21, 2025
13 checks passed
@fwcd
Copy link
Member

fwcd commented Feb 22, 2025

It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future.

I just imagined the hypothetical scenario where we wanted to distribute Mixxx in the App Store again and couldn't because this was the only contribution where the author didn't sign the CLA 😄

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.

4 participants