-
Notifications
You must be signed in to change notification settings - Fork 80
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: Add bindings for QMessageLogContext and qt_message_output #814
base: main
Are you sure you want to change the base?
Conversation
Hi, |
5b47aee
to
c8364f2
Compare
c8364f2
to
f9950b7
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.
Good first pass, unfortunately the lifetimes need sorting out.
40efe50
to
7dc5dbb
Compare
f519be1
to
78b72af
Compare
bff8d3a
to
88eaa9e
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #814 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 71 71
Lines 11967 11967
=========================================
Hits 11967 11967 ☔ View full report in Codecov by Sentry. |
d204d89
to
9e0cd75
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.
The lifetimes are tricky to get right here and still need sorting out.
I also unresolved my two previous comments as both still need more work unfortunately.
61dc340
to
6b03a11
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.
I'm pretty happy with this PR now.
Having static_assertions would be a plus. Waiting for opinion from @ahayzen-kdab there.
6b03a11
to
1e8003a
Compare
1e8003a
to
438b6c4
Compare
This allows Rust and CXX-Qt applications to send messages to the Qt logger.
438b6c4
to
7326ea0
Compare
This is needed to ensure the Rust-side QMessageLogContext struct has the expected size.
7326ea0
to
68454a7
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.
LGTM, @redstrate please rebase, then we can merge.
This allows Rust and CXX-Qt applications to send messages to the Qt logger.