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.

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

Clang.jl generated bindings and internal refactor #296

Merged
merged 4 commits into from
Aug 31, 2022

Conversation

metab0t
Copy link
Collaborator

@metab0t metab0t commented Jun 10, 2022

A first step of #295

  1. Use Clang.jl to generate C bindings automatically
  2. Use WeakKeyDict to record prepared statements of a DB
  3. Upgrade Julia version requirement to 1.6

All tests pass locally.

@metab0t
Copy link
Collaborator Author

metab0t commented Jun 10, 2022

@quinnj
JuliaFormatter.jl causes extra diff but the runtests.jl is almost unaffected (no public API change).
So maybe we can merge it as long as CI is green?

@metab0t
Copy link
Collaborator Author

metab0t commented Jun 10, 2022

The formatter causes a lot of noisy diffs, I can open another PR to format existing code as Step 0 then merge this PR if we agree on the formatter style configuration.

1. Use Clang.jl to generate C bindings automatically
2. Use JuliaFormatter.jl to format all the code
3. Use WeakKeyDict to record prepared statements of a DB
4. Upgrade Julia version requirement to 1.6
@metab0t metab0t force-pushed the capi_and_internal_refactor branch from 6f9ae6a to 14e47d9 Compare August 31, 2022 08:21
@codecov
Copy link

codecov bot commented Aug 31, 2022

Codecov Report

Merging #296 (14e47d9) into master (a4e4da3) will decrease coverage by 30.28%.
The diff coverage is 93.10%.

@@             Coverage Diff             @@
##           master     #296       +/-   ##
===========================================
- Coverage   85.88%   55.59%   -30.29%     
===========================================
  Files           5        5               
  Lines         758     1108      +350     
===========================================
- Hits          651      616       -35     
- Misses        107      492      +385     
Impacted Files Coverage Δ
src/capi.jl 15.69% <ø> (ø)
src/UDF.jl 86.36% <86.20%> (+0.20%) ⬆️
src/SQLite.jl 95.94% <93.69%> (+1.88%) ⬆️
src/tables.jl 99.34% <96.55%> (-0.01%) ⬇️
src/base.jl 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@metab0t
Copy link
Collaborator Author

metab0t commented Aug 31, 2022

@quinnj
Now #297 has merged. The diff is nicer to review except for 3k+ lines added by automatically generated C API bindings 😂

@metab0t metab0t changed the title Refactor Step 1 Clang.jl generated bindings and internal refactor Aug 31, 2022
Copy link
Member

@quinnj quinnj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great; thanks @metab0t!

@quinnj quinnj merged commit e7264a9 into JuliaDatabases:master Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants