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

quickinspector: Use QT_VERSION_MAJOR #920

Closed
wants to merge 1 commit into from
Closed

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Jan 14, 2024

In ac8ed0c I fixed this in plugins/qmlsupport/CMakeLists.txt, but I somehow missed the exact same issue in plugins/quickinspector/CMakeLists.txt.

Qt target names should be constructed using Qt${QT_VERSION_MAJOR}::target, not Qt${QT_MAJOR_VERSION}::target, to conform with the rest of the build. (QT_MAJOR_VERSION is an ECM thing, and is only found should only be found in the files in cmake/ECM/modules/.)

@dantti
Copy link
Member

dantti commented Jan 14, 2024

This is a duplicate of #916 but was not commited due internal conversation that ECM uses the current form, so far both options work, maybe due qt versionless targets or that this doesn't matter, still CI is happy so I would merge as other files use this form.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Jan 14, 2024

Never mind, @dantti already caught this in #916.

@ferdnyc ferdnyc closed this Jan 14, 2024
@ferdnyc
Copy link
Contributor Author

ferdnyc commented Jan 14, 2024

@dantti Heh. Crossed streams, I discovered your PR at the exact moment you commented.

@ferdnyc ferdnyc deleted the cmake-var branch January 14, 2024 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants