-
Notifications
You must be signed in to change notification settings - Fork 15
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
Copy code from dbt core #13
Conversation
0c380be
to
e5f7b08
Compare
e5f7b08
to
243ac3c
Compare
3eb228a
to
9c69956
Compare
Add CI workflow
21a08ef
to
009e971
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 left a few comments. Most were around the idea that we might want to move some of this to dbt-adapter
if it's specific to an adapter. In particular, if the primary functionality lives there (e.g. connection
), then any associated functionality should also live there (or we need to move the primary back to dbt-common
).
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.
just nitpicking otherwise LGTM
fwiw, I'm able to build |
Update, I migrated all unit tests to |
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.
Approving assuming the cache change goes through as discussed separately.
Resolves: dbt-labs/dbt-core#10
Acceptance Criteria
Follow-up work: