-
Notifications
You must be signed in to change notification settings - Fork 84
cxx-qt-build: return an Interface from CxxQtBuilder #1287
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
base: main
Are you sure you want to change the base?
cxx-qt-build: return an Interface from CxxQtBuilder #1287
Conversation
8f18f8a
to
6a6c7c0
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1287 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 75 75
Lines 12772 12772
=========================================
Hits 12772 12772 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9f8bd4e
to
4bb39aa
Compare
Overall looking good, does now need a rebase now the other branch has landed, unsure if it'll conflict in any way. |
This still needs more refactoring later but is a first step.
This allows export() to have no args meaning we can return an Interface from the compile command next.
We however have two changes compared to CXX: 1. We allow you to change the root of the automatic include directory. 2. We allow you to include additional directories. Both of these changes are there to accomodate crates like cxx-qt-lib, that want to use e.g. `<cxx-qt-lib/qstring.h>` instead of `<cxx-qt-lib/include/qstring.h>`. Also cxx-qt-lib generates a definitions header which we need to include.
This is no longer needed. Also clean up some documentation on the API.
4bb39aa
to
f0c7317
Compare
The function will now panic if it is called multiple times.
8245fd7
to
adc3164
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed think this is a good step in the right direction, didn't know if you want to use this in it's current state or keep working on it. Going to approve so you can land this if required and iterate separately :-)
Another take on #1285.
The main difference is in how include paths are handled for the CXX-style automatic include of the header directory.
In this PR, the root of the automatic inclue can be configured.
This leads to less changes for downstream crates.
It also allows us to move to an include system of "cxx-qt-lib/core/qstring.h" while keeping backwards-compatibility with forwarding-headers