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

cxx-qt-lib: require headers to be includes manually #764

Merged
merged 8 commits into from
Feb 21, 2024

Conversation

ahayzen-kdab
Copy link
Collaborator

This allows for cxx-qt-lib to be optional later.

Related to #319

Cargo.toml Outdated Show resolved Hide resolved
@ahayzen-kdab ahayzen-kdab force-pushed the 319-make-cxx-qt-lib-optional branch from 1edf708 to 8b24374 Compare December 6, 2023 15:51
@ahayzen-kdab ahayzen-kdab force-pushed the 319-make-cxx-qt-lib-optional branch 4 times, most recently from 05b6d0d to 12895a7 Compare January 17, 2024 18:29
@ahayzen-kdab
Copy link
Collaborator Author

Not sure why Windows is failing with a missing dll 🤔

@ahayzen-kdab ahayzen-kdab force-pushed the 319-make-cxx-qt-lib-optional branch 2 times, most recently from 43ef1b0 to d79eff6 Compare February 6, 2024 07:56
@ahayzen-kdab ahayzen-kdab force-pushed the 319-make-cxx-qt-lib-optional branch 2 times, most recently from 1e1c17b to 378fdab Compare February 7, 2024 07:44
@ahayzen-kdab ahayzen-kdab force-pushed the 319-make-cxx-qt-lib-optional branch 2 times, most recently from 732ace9 to f4540ce Compare February 8, 2024 05:05
@ahayzen-kdab ahayzen-kdab marked this pull request as ready for review February 8, 2024 05:16
@ahayzen-kdab ahayzen-kdab force-pushed the 319-make-cxx-qt-lib-optional branch 2 times, most recently from 0fd4513 to e1c0fe0 Compare February 8, 2024 07:22
@ahayzen-kdab ahayzen-kdab force-pushed the 319-make-cxx-qt-lib-optional branch from 48d0456 to 17f8d15 Compare February 16, 2024 12:07
Copy link
Collaborator

@LeonMatthesKDAB LeonMatthesKDAB left a comment

Choose a reason for hiding this comment

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

Overall direction looks good.

I think the Opts struct needs more encapsulation though.
Some of the features can now also be removed maybe?

crates/cxx-qt-build/src/lib.rs Outdated Show resolved Hide resolved
crates/cxx-qt-build/Cargo.toml Outdated Show resolved Hide resolved
crates/cxx-qt-lib-headers/Cargo.toml Outdated Show resolved Hide resolved
crates/cxx-qt-lib/build.rs Outdated Show resolved Hide resolved
@ahayzen-kdab ahayzen-kdab force-pushed the 319-make-cxx-qt-lib-optional branch 4 times, most recently from 39e5fe7 to 732d692 Compare February 21, 2024 11:18
This allows for cxx-qt-lib to be optional later.

Related to KDAB#319
When cxx-qt-lib is a build dependency we have a failure with
a missing dll.
As this is how cxx bridges normally function, ideally we want
this for cxx-qt bridges too but the macro needs to know the file
name too.
This then means we can remove the gui and qml features
@ahayzen-kdab ahayzen-kdab force-pushed the 319-make-cxx-qt-lib-optional branch from 732d692 to 4774699 Compare February 21, 2024 12:08
Copy link
Collaborator

@LeonMatthesKDAB LeonMatthesKDAB left a comment

Choose a reason for hiding this comment

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

LGTM

@LeonMatthesKDAB LeonMatthesKDAB merged commit 1b2d643 into KDAB:main Feb 21, 2024
10 checks passed
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