Skip to content

cxx-qt-gen: move headers to cxx-qt crate #843

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
Merged
merged 9 commits into from
Feb 21, 2024
Merged

cxx-qt-gen: move headers to cxx-qt crate #843

merged 9 commits into from
Feb 21, 2024

Conversation

ahayzen-kdab
Copy link
Collaborator

@ahayzen-kdab ahayzen-kdab commented Feb 8, 2024 •
edited
Loading

Closes #844

@ahayzen-kdab ahayzen-kdab force-pushed the 319-move-headers-to-cxx-qt branch 4 times, most recently from 34a3ed7 to a146a59 Compare February 12, 2024 05:44
@ahayzen-kdab ahayzen-kdab marked this pull request as ready for review February 12, 2024 05:46
@ahayzen-kdab ahayzen-kdab mentioned this pull request Feb 12, 2024
Merged
@ahayzen-kdab ahayzen-kdab force-pushed the 319-move-headers-to-cxx-qt branch from a146a59 to 8e5a4b7 Compare February 12, 2024 07:23
@ahayzen-kdab ahayzen-kdab mentioned this pull request Feb 12, 2024
Merged
@ahayzen-kdab ahayzen-kdab force-pushed the 319-move-headers-to-cxx-qt branch 2 times, most recently from 1817613 to 7c821a1 Compare February 16, 2024 16:15
@ahayzen-kdab ahayzen-kdab force-pushed the 319-move-headers-to-cxx-qt branch from 7c821a1 to 54559b7 Compare February 16, 2024 16:27
@ahayzen-kdab ahayzen-kdab force-pushed the 319-move-headers-to-cxx-qt branch 2 times, most recently from 6dff876 to 520bf06 Compare February 16, 2024 17:36
@ahayzen-kdab ahayzen-kdab force-pushed the 319-move-headers-to-cxx-qt branch from 520bf06 to c641bcf Compare February 16, 2024 17:59
@ahayzen-kdab
Copy link
Collaborator Author

Windows CI broke on this branch this time with the missing DLL, hoping it was related to add cxx-qt as a dependency to qt_types_standalone ...

@ahayzen-kdab
Copy link
Collaborator Author

Windows CI broke on this branch this time with the missing DLL, hoping it was related to add cxx-qt as a dependency to qt_types_standalone ...

Nope still fails, on my CI instance https://github.com/ahayzen-kdab/cxx-qt/actions it can be seen that when the commit chain including rename to cxx-qt-common to cxx-qt is when it started to occur. So will need to diff between ahayzen-kdab@7c821a1 and ahayzen-kdab@54559b7

@ahayzen-kdab
Copy link
Collaborator Author

The diff is quite small, this is when the guard was moved to cxx-qt and added as a bridge that we might not require. So is the whole issue of the missing DLL on Windows something to do with extern "Rust" blocks ? (Note we don't even potentially require this)

@ahayzen-kdab ahayzen-kdab force-pushed the 319-move-headers-to-cxx-qt branch from e18904a to 637fce3 Compare February 16, 2024 18:22
@ahayzen-kdab
Copy link
Collaborator Author

So yes turns out that the extern "Rust" in cxx-qt was causing the build failures of a missing DLL on Windows.

@ahayzen-kdab ahayzen-kdab mentioned this pull request Feb 16, 2024
Closed
@ahayzen-kdab ahayzen-kdab force-pushed the 319-move-headers-to-cxx-qt branch 2 times, most recently from 855dfb7 to 2a83381 Compare February 19, 2024 12:54
@ahayzen-kdab ahayzen-kdab force-pushed the 319-move-headers-to-cxx-qt branch from 2a83381 to 26cfc0f Compare February 21, 2024 08:33
LeonMatthesKDAB
LeonMatthesKDAB approved these changes Feb 21, 2024
View reviewed changes
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.

Looks good :)

Great change!

@LeonMatthesKDAB LeonMatthesKDAB enabled auto-merge (rebase) February 21, 2024 09:29
@LeonMatthesKDAB LeonMatthesKDAB merged commit 5c7a71d into KDAB:main Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeonMatthesKDAB LeonMatthesKDAB LeonMatthesKDAB approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Investigate moving signals to return a normal type then use a manual guard
2 participants
@ahayzen-kdab @LeonMatthesKDAB