-
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-gen: add support for passing in CfgEvaluator #1168
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1168 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 71 73 +2
Lines 11882 12426 +544
==========================================
+ Hits 11882 12426 +544 ☔ View full report in Codecov by Sentry. |
9e223b7
to
3fe67ce
Compare
3fe67ce
to
1673abc
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.
Great work overall.
I thinkg this is lacking a snapshot test though, so it's easier to see where cfg attributes are now enabled and how that changes the generation.
Also, cfg attributes on QObject/QEnum don't seem to be respected yet.
We're also missing a CHANGELOG entry 😉
9359ce5
to
fe7f8a2
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 test output looks pretty good.
Enabled items on disabled types may still be an issue.
They will fail to compile, which is expected, but the error messages are likely to be cryptic.
If there is an easy way to give a nicer error message we should probably do so.
However, if it's hard to give a good error message, we can keep it as-is.
And may confuse the results.
This avoids collisions between paths, we were seeing odd issues around some files having a different tracked amount in codecov vs lcov. It appears to be due to having coverage results from cxx_gen with the same file paths. We do not want coverage data for anything else anyway so filter them out.
fe7f8a2
to
69cd338
Compare
69cd338
to
729a56e
Compare
No description provided.