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

Ability to skip when both decks are playing on auto dj #2531

Merged
merged 5 commits into from
Mar 8, 2020
Merged

Ability to skip when both decks are playing on auto dj #2531

merged 5 commits into from
Mar 8, 2020

Conversation

crisclacerda
Copy link

@crisclacerda crisclacerda commented Mar 5, 2020

This PR fixes this https://bugs.launchpad.net/mixxx/+bug/1399974?comments=all

Now when 2 decks are playing it assumes the user wants to remove the next track on the auto dj playlist.

@daschuer
Copy link
Member

daschuer commented Mar 5, 2020

Thank you for your first PR.

Unfortunately a commit hook on Travis fails:

Trim Trailing Whitespace.................................................Failed

You may also install the commit hooks locally if you like:
https://github.com/mixxxdj/mixxx/blob/master/.pre-commit-config.yaml
Unfortunately this was only tested on Linux, so you may discover issue on MacOs.

Unfortunately another test fails as well:

206 - EngineMicrophoneTest.TestInputMatchesOutput (Child aborted) 

Likely a false positive, so we can ignore it here. I sorry about that. Our CI is not that reliably. We need to work on it but that is another topic.

@daschuer
Copy link
Member

daschuer commented Mar 5, 2020

The code looks good to me (LGTM). Thank you.

If you have a Launchpad account you can assign the bug to yourself.

Before merge, we need your permission.
Please sign:
https://docs.google.com/a/mixxx.org/spreadsheet/viewform?formkey=dEpYN2NkVEFnWWQzbkFfM0ZYYUZ5X2c6MQ

and comment here when done.

@Holzhaus
Copy link
Member

Holzhaus commented Mar 5, 2020

@daschuer The patch looks pretty small. Do you think it makes sense to backport this fix for 2.2.4?

@daschuer
Copy link
Member

daschuer commented Mar 6, 2020

Yes probably.
@

@daschuer daschuer closed this Mar 6, 2020
@daschuer daschuer reopened this Mar 6, 2020
@daschuer
Copy link
Member

daschuer commented Mar 6, 2020

Sorry the close and comment button strikes me again.
@crisclacerda please rebase your branch to 2.2
And forth push it. This will update this PR.
Than we can change the target branch to 2.2

@Holzhaus
Copy link
Member

Holzhaus commented Mar 6, 2020

In case you don't know how to rebase: https://mixxx.org/wiki/doku.php/using_git#targeting_another_base_branch

@uklotzde
Copy link
Contributor

uklotzde commented Mar 6, 2020

git rebase --onto 2.2 49372742c5637a71b833d84695c95aaa8b775608^

@crisclacerda crisclacerda changed the base branch from master to 2.2 March 6, 2020 17:33
@crisclacerda
Copy link
Author

Made changes, commented on launchpad post and signed document.

@crisclacerda
Copy link
Author

Hm, it failed again to build on CI. Now it looks like it's trying to use python 3 when building on mac OS.
Sorry about that, I am not familiar with these systems.

File "/Users/travis/build/mixxxdj/mixxx/build/osx/OSConsX.py", line 36
    print s,
          ^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print(s, end=" ")?

@@ -234,6 +234,18 @@ AutoDJProcessor::AutoDJError AutoDJProcessor::skipNext() {
} else if (!rightDeck.isPlaying()) {
removeLoadedTrackFromTopOfQueue(rightDeck);
loadNextTrackFromQueue(rightDeck);
} else {
Copy link
Member

Choose a reason for hiding this comment

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

Here is the tailing white space error, Travis complains about.

@daschuer
Copy link
Member

daschuer commented Mar 6, 2020

I think in this case we can ignore most of the CI build issues. This is all a bit legacy.
I am just building locally and if it works successful we can merge.

@uklotzde
Copy link
Contributor

uklotzde commented Mar 7, 2020

The 2.2 CI builds are broken depending on the platform and won't be fixed. These are known issues. We need to abandon the 2.2 branch with a final release asap.

@uklotzde
Copy link
Contributor

uklotzde commented Mar 7, 2020

Please add a descriptive line with a reference to the bug to the CHANGELOG

@daschuer
Copy link
Member

daschuer commented Mar 8, 2020

LGTM Thank you.

@daschuer daschuer merged commit b127ebf into mixxxdj:2.2 Mar 8, 2020
@crisclacerda
Copy link
Author

Nice, thanks for guiding me through my first PR and sorry for any inconvenience, hopefully, next ones will be more smooth =)

@daschuer
Copy link
Member

daschuer commented Mar 8, 2020

To be honest, this was quite smooth, every complains where of type tap doors.
I am sorry for that. Carry on. :-)

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