MADR decision record for moving cross-database macros into dbt-core + adapters #615
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joellabes
reviewed
Jul 7, 2022
There was a problem hiding this comment.
This looks pretty good! It doesn't reflect the decision to move the load_by_period macro into the experimental features repo though. So in reality we made several decisions:
- Keep tests and macros together
- Move experiments to their own repo
- Move cross-db macros to Core/adapters
| * Good, because common, reusable functionality that differs across databases will work "out of the box" | ||
| * Good, because functionality can subjected to more [rigorous testing](https://docs.getdbt.com/docs/contributing/testing-a-new-adapter) | ||
| * Good, because we hope that many package vendors could drop their dependencies on `dbt_utils` altogether, which makes version resolution easier | ||
| * Good, because it's slightly more convenient to reference the macro as `dateadd` instead of `dbt_utils.dateadd` |
There was a problem hiding this comment.
I think I intend to always document them as dbt.dateadd() so that it's clear that they're coming from somewhere as opposed to magic macros, so this is maybe a neutral IMO
|
I like the DACI table at the top of the page 😍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a:
All pull requests from community contributors should target the
mainbranch (default).Description & motivation
See ADR-0002 for details.