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

cxx-qt-build: consider the folders in rust path #856

Merged
merged 15 commits into from
Sep 10, 2024
Merged

cxx-qt-build: consider the folders in rust path #856

merged 15 commits into from
Sep 10, 2024

Conversation

ahayzen-kdab
Copy link
Collaborator

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

Related to #855

@ahayzen-kdab ahayzen-kdab force-pushed the 855-use-folders branch 3 times, most recently from 4fe8ce3 to bd6d571 Compare April 18, 2024 15:05
@ahayzen-kdab ahayzen-kdab marked this pull request as ready for review April 18, 2024 15:05
@ahayzen-kdab ahayzen-kdab changed the title WIP: cxx-qt-build: consider the folders in rust path cxx-qt-build: consider the folders in rust path Apr 18, 2024
@ahayzen-kdab ahayzen-kdab marked this pull request as draft April 18, 2024 15:37
@ahayzen-kdab ahayzen-kdab mentioned this pull request May 20, 2024
Closed
@ahayzen-kdab ahayzen-kdab force-pushed the 855-use-folders branch 2 times, most recently from 38f44c2 to 87270e6 Compare August 23, 2024 11:55
@ahayzen-kdab ahayzen-kdab force-pushed the 855-use-folders branch 3 times, most recently from 4e950ee to b07ce66 Compare September 4, 2024 15:48
@ahayzen-kdab ahayzen-kdab marked this pull request as ready for review September 4, 2024 15:50
@codecov Codecov
Copy link

codecov bot commented Sep 4, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (455bcf4) to head (7b6cfc7).

Additional details and impacted files 
@@            Coverage Diff            @@
##              main      #856   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           71        70    -1     
  Lines        12030     11991   -39     
=========================================
- Hits         12030     11991   -39

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ahayzen-kdab ahayzen-kdab force-pushed the 855-use-folders branch 2 times, most recently from 64bc715 to 0e36c7a Compare September 6, 2024 09:29
LeonMatthesKDAB
LeonMatthesKDAB requested changes Sep 6, 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.

Awesome that we can finally get this to work!

I'm slightly concerned that the current include path setup may lead to conflicts though, especially when adding multiple qml modules.

crates/cxx-qt-build/Cargo.toml Outdated Show resolved Hide resolved
crates/cxx-qt-build/src/lib.rs Outdated Show resolved Hide resolved
crates/cxx-qt-build/src/lib.rs Outdated Show resolved Hide resolved
crates/cxx-qt-build/src/lib.rs Outdated Show resolved Hide resolved
crates/cxx-qt-build/src/lib.rs Outdated Show resolved Hide resolved
crates/cxx-qt-build/src/lib.rs Outdated Show resolved Hide resolved
@ahayzen-kdab
Copy link
Collaborator Author

Weird it compiles locally for me in both release and debug 😱 and it did work before with CI 🤔

@ahayzen-kdab ahayzen-kdab force-pushed the 855-use-folders branch 2 times, most recently from 02dff8b to d0e2a20 Compare September 9, 2024 10:31
@ahayzen-kdab
Copy link
Collaborator Author

Found it was the cargo build failing, now that is fixed just cargo test now fails :-/

@ahayzen-kdab ahayzen-kdab mentioned this pull request Sep 10, 2024
Open
@ahayzen-kdab
cxx-qt-build: consider the folders in rust path
dc56b45 
Closes KDAB#855
@ahayzen-kdab
cxx-qt-gen: require cxx_file_stem until Span::source_file is stable
b7ab96e 
Closes KDAB#855
@ahayzen-kdab
cxx-qt-gen: change writer phase to take include path from cxx-qt-build
cd523e0
@ahayzen-kdab
cxx-qt-build: place moc and other files into folder and include moc
69c47a1
@ahayzen-kdab
cxx-qt-build: injected include needs the prefix of the crate
8a1cc85
@ahayzen-kdab
cxx-qt-gen: purge cxx_file_stem
6a4e353
@ahayzen-kdab
cxx-qt-gen: fix failing tests
6d64e48
@ahayzen-kdab
book: remove line about cxx_file_stem and Span::source_file()
226d363
@ahayzen-kdab
cxx-qt-gen: add test to ensure cxx_file_stem fails
a91fd6f
@ahayzen-kdab
errors: improve error messages for cxx_file_stem and multiple dirs
f1a48bc
@ahayzen-kdab
cxx-qt-build: remove use of syn it is not used
330e7e5
@ahayzen-kdab
cxx-qt-build: use a cloned builder for qml modules
ec9e0b8
@ahayzen-kdab
cxx-qt-build: use full directory path when comparing qml roots
1c6a3d6
@ahayzen-kdab
cxx-qt-build: use only link when building
9dfc06d
@ahayzen-kdab
cxx-qt-build: use same builder for now for all qml modules
7b6cfc7
LeonMatthesKDAB
LeonMatthesKDAB approved these changes Sep 10, 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.

LGTM :)

@ahayzen-kdab ahayzen-kdab merged commit 900d193 into KDAB:main Sep 10, 2024
14 checks passed
@ahayzen-kdab ahayzen-kdab mentioned this pull request Oct 4, 2024
Closed
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.
2 participants
@ahayzen-kdab @LeonMatthesKDAB