-
Notifications
You must be signed in to change notification settings - Fork 3
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
Feature/add union schema #19
Conversation
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.
@fivetran-reneeli thanks for applying these updates. This PR looks good to go with the following callouts.
- Be sure to address the notes below before release review.
- Please confirm the integrity test still passes with the join adjustment.
- Please complete the PR template for posterity.
CHANGELOG.md
Outdated
|
||
## Documentation | ||
- Added Quickstart model counts to README. ([#18](https://github.com/fivetran/dbt_youtube_analytics/pull/18)) | ||
- Corrected references to connectors and connections in the README. ([#18](https://github.com/fivetran/dbt_youtube_analytics/pull/18)) | ||
|
||
## Under the Hood | ||
- Updated the source table references in order to execute the union macro successfully. |
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.
Same comment as the source PR - Let's call this out as a breaking change and highlight the specific source name changes.
packages.yml
Outdated
# - package: fivetran/youtube_analytics_source | ||
# version: [">=0.5.0", "<0.6.0"] | ||
|
||
- git: https://github.com/fivetran/dbt_youtube_analytics_source.git | ||
revision: feature/add_union_schema | ||
warn-unpinned: false |
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.
Reminder to switch before merge
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.
Looks great - same minor comments as on the source package
Co-authored-by: Jamie Rodriguez <65564846+fivetran-jamie@users.noreply.github.com>
Co-authored-by: Jamie Rodriguez <65564846+fivetran-jamie@users.noreply.github.com>
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.
Approved with one final README suggestion
Co-authored-by: Jamie Rodriguez <65564846+fivetran-jamie@users.noreply.github.com>
PR Overview
This PR will address the following Issue/Feature: adding source_relation and #14
Submission Checklist
Submitter:
Testing the union macro and results of validation tests provided in internal ticket
See internal ticket
Reviewer:
Changelog