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

Feature/add union schema #19

Merged
merged 17 commits into from
Mar 11, 2025
Merged

Feature/add union schema #19

merged 17 commits into from
Mar 11, 2025

Conversation

fivetran-reneeli
Copy link
Contributor

@fivetran-reneeli fivetran-reneeli commented Feb 27, 2025

PR Overview

This PR will address the following Issue/Feature: adding source_relation and #14

Submission Checklist

Submitter:

  • Alignment meeting with the reviewer
  • Provide validation details:
    • Validation Steps: Outline how to verify the changes

Testing the union macro and results of validation tests provided in internal ticket

  • Testing Instructions: Clear steps for running/testing (e.g., scripts, sample data)

See internal ticket

  • [na] Focus Areas: Highlight any complex logic or queries needing special attention

Reviewer:

  • Confirm submission requirements are met

Changelog

  • Draft after PR approval

Copy link
Collaborator

@fivetran-joemarkiewicz fivetran-joemarkiewicz left a 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.
Copy link
Collaborator

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
Comment on lines 2 to 7
# - 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
Copy link
Collaborator

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

Copy link
Contributor

@fivetran-jamie fivetran-jamie left a 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

@fivetran-jamie fivetran-jamie self-requested a review March 7, 2025 19:29
fivetran-reneeli and others added 3 commits March 10, 2025 11:50
Co-authored-by: Jamie Rodriguez <65564846+fivetran-jamie@users.noreply.github.com>
Co-authored-by: Jamie Rodriguez <65564846+fivetran-jamie@users.noreply.github.com>
Copy link
Contributor

@fivetran-jamie fivetran-jamie left a 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

fivetran-reneeli and others added 2 commits March 10, 2025 16:41
Co-authored-by: Jamie Rodriguez <65564846+fivetran-jamie@users.noreply.github.com>
@fivetran-reneeli fivetran-reneeli merged commit 3e31531 into main Mar 11, 2025
9 checks passed
@fivetran-reneeli fivetran-reneeli deleted the feature/add_union_schema branch March 11, 2025 21:00
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