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

Clean up MixxxMainWindow and RecordingManager #2872

Merged
merged 5 commits into from
Jul 12, 2020
Merged

Clean up MixxxMainWindow and RecordingManager #2872

merged 5 commits into from
Jul 12, 2020

Conversation

xeruf
Copy link
Contributor

@xeruf xeruf commented Jun 15, 2020 •
edited
Loading

For some reason pre-commit decided to reformat both files completely, even though I only intended to fix the headers, includes and remove superfluous comments - oh well ^^

@daschuer
Copy link
Member

I have no idea why pre-commit does not work in this case.
Please redo the minimal commit and and commit it with --no-verify.
This can than be used to fix the pre-commit issues and helps to get this one merged.

daschuer
daschuer requested changes Jun 15, 2020
View reviewed changes
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

This PR has unrelated changes.

@Be-ing
Copy link
Contributor

Be-ing commented Jun 15, 2020

Please don't mass reformat mixxx.cpp. This will likely cause unnecessary and annoying merge conflicts with open PRs.

@xeruf
Copy link
Contributor Author

xeruf commented Jun 16, 2020

Alright, compartmentalized it and cleaned up :)

@xeruf xeruf requested a review from daschuer June 16, 2020 10:46
@xeruf
Copy link
Contributor Author

xeruf commented Jun 18, 2020

@daschuer please have another look :)

daschuer
daschuer requested changes Jun 19, 2020
View reviewed changes
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

Tank you for the rework. Here some comments.

src/mixxx.h Outdated Show resolved Hide resolved
src/mixxx.cpp Outdated Show resolved Hide resolved
src/mixxx.h Outdated Show resolved Hide resolved
@xeruf xeruf requested review from daschuer and Holzhaus June 20, 2020 21:09
@xeruf
Copy link
Contributor Author

xeruf commented Jul 2, 2020

All resolved :)

Holzhaus
Holzhaus approved these changes Jul 12, 2020
View reviewed changes
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.

LGTM, CI fails are unrelated.

@Holzhaus Holzhaus merged commit d6e4f3b into mixxxdj:master Jul 12, 2020
@Holzhaus Holzhaus added this to the 2.4.0 milestone Jul 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Holzhaus Holzhaus Holzhaus approved these changes

@daschuer daschuer Awaiting requested review from daschuer

Assignees
No one assigned
Labels
code quality
Projects
2.4 release
Status: Done
Milestone
2.4.0
Development

Successfully merging this pull request may close these issues.
4 participants
@xeruf @daschuer @Be-ing @Holzhaus