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

refactor: fetch extensions #681

Merged
merged 15 commits into from
Mar 12, 2025
Merged

refactor: fetch extensions #681

merged 15 commits into from
Mar 12, 2025

Conversation

gadomski
Copy link
Member

@gadomski gadomski commented Mar 12, 2025

@ceholden this is a bit of a simpler implementation. I wasn't finding a fetch_extensions setup that felt right to me. I tested locally by deleting my extensions and running just the no_install test and it failed 🥳

@gadomski gadomski self-assigned this Mar 12, 2025
@codecov-commenter
Copy link

codecov-commenter commented Mar 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 42.02%. Comparing base (973414b) to head (7c9d5ca).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #681      +/-   ##
==========================================
+ Coverage   41.98%   42.02%   +0.03%     
==========================================
  Files          47       47              
  Lines        2751     2751              
==========================================
+ Hits         1155     1156       +1     
+ Misses       1596     1595       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@ceholden ceholden left a comment

Choose a reason for hiding this comment

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

+1 to the design in general, especially as the DuckDB config has all of these in the same place

@gadomski gadomski requested review from ceholden March 12, 2025 17:09
Copy link
Collaborator

@ceholden ceholden left a comment

Choose a reason for hiding this comment

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

Thanks! I think this looks great

@gadomski gadomski merged commit 1888a15 into main Mar 12, 2025
12 checks passed
@gadomski gadomski deleted the refactor-fetch-extensions branch March 12, 2025 17:21
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.

3 participants