-
Notifications
You must be signed in to change notification settings - Fork 85
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
Refactor exception.h #2006
Refactor exception.h #2006
Conversation
b3e867a
to
1b5ffdb
Compare
1b5ffdb
to
d256e3a
Compare
d256e3a
to
6bfe5db
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #2006 +/- ##
=======================================
Coverage 90.09% 90.10%
=======================================
Files 919 934 +15
Lines 33257 33258 +1
=======================================
+ Hits 29964 29967 +3
+ Misses 3293 3291 -2
☔ View full report in Codecov by Sentry. |
7802c74
to
3abaf90
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. Thanks!
Let's rebase and get the benchmark CI passed before merge.
Please make sure that the header merge script still works (you can trigger https://github.com/kuzudb/kuzu/actions/workflows/linux-precompiled-bin-workflow.yml and test the generated lib + header) before merging. |
@mewim how do I test the generated lib + header? |
This was the most included header in the codebase. This refactoring should help compile times.
3abaf90
to
a31272a
Compare
You can follow the example in the documentation: https://kuzudb.com/docusaurus/getting-started/cpp The dataset used is under https://github.com/kuzudb/kuzu/tree/master/dataset/demo-db/csv |
Tested against the example linked, works. Benchmark is now passing. I'm rerunning MSVC that hit a timeout of some kind while testing, and will merge after that. |
This was the most included header in the codebase. This refactoring should help compile times.