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

remove compatibility.h #4309

Merged
merged 2 commits into from
Sep 19, 2021
Merged

remove compatibility.h #4309

merged 2 commits into from
Sep 19, 2021

Conversation

Be-ing
Copy link
Contributor

@Be-ing Be-ing commented Sep 19, 2021

None of it is needed anymore.

@Be-ing Be-ing mentioned this pull request Sep 19, 2021
Merged
@uklotzde
Copy link
Contributor

I don't think so. We still need to support Qt 5.12.

@Be-ing
Copy link
Contributor Author

Be-ing commented Sep 19, 2021

Oh I did not realize QWidget::screen was added in Qt 5.14. I can add one conditional hack for that, but I think we can still get rid of compatibility.h.

@uklotzde
Copy link
Contributor

Please don't remove the QUuid utility functions and tests, as these will still be needed in the future, e.g. for #2282.

@uklotzde
Copy link
Contributor

I suggest to first merge #4308 and then continue with the remaining tasks.

@Be-ing
Copy link
Contributor Author

Be-ing commented Sep 19, 2021

I don't think we need to have unit tests for Qt. That's Qt's job.

@uklotzde
Copy link
Contributor

uklotzde commented Sep 19, 2021 •
edited
Loading

I don't think we need to have unit tests for Qt. That's Qt's job.

Qt doesn't provide a function that maps a null UUID to a null string. I have already populated some of my tags with those bogus UUIDs, don't underestimate even those simple functions.

@uklotzde
Copy link
Contributor

I have removed the obsolete utility function and tests from #4308.

@Be-ing
remove getDevicePixelRatioF compatibility function
2836a4f 
This was added for Qt < 5.6 but the conditional compilation supporting
Qt < 5.6 was already removed. The function no longer serves a purpose.
@Be-ing Be-ing force-pushed the devicepixelratio_qt56 branch 2 times, most recently from 34d5ae6 to dce1aad Compare September 19, 2021 17:52
@Be-ing
Copy link
Contributor Author

Be-ing commented Sep 19, 2021

Rebased and added one special case hack for Qt < 5.14.

@Be-ing Be-ing force-pushed the devicepixelratio_qt56 branch 3 times, most recently from aebc83d to d82c367 Compare September 19, 2021 18:20
uklotzde
uklotzde suggested changes Sep 19, 2021
View reviewed changes
Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

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

devicePixelRatio() -> devicePixelRatioF()

@Be-ing
remove obsolete getPrimaryScreen compatibility function
2f08b4f 
This was added for Qt < 5.6 but the conditional compilation supporting
Qt < 5.6 was already removed. The function no longer serves a purpose.

Also remove compatibility.h
@Be-ing
Copy link
Contributor Author

Be-ing commented Sep 19, 2021

Merge?

@uklotzde
Copy link
Contributor

LGTM

@uklotzde uklotzde merged commit 4bf6196 into mixxxdj:main Sep 19, 2021
@Be-ing Be-ing deleted the devicepixelratio_qt56 branch September 19, 2021 19:39
@Be-ing
Copy link
Contributor Author

Be-ing commented Sep 19, 2021

Thank you for the speedy review.

@daschuer daschuer mentioned this pull request Dec 12, 2021
Merged
@JoergAtGithub JoergAtGithub mentioned this pull request Dec 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uklotzde uklotzde uklotzde approved these changes

Assignees
No one assigned
Labels
build code quality library ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Be-ing @uklotzde