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 data #44

Merged
merged 26 commits into from
Dec 2, 2024
Merged

Feature/add union data #44

merged 26 commits into from
Dec 2, 2024

Conversation

fivetran-jamie
Copy link
Contributor

@fivetran-jamie fivetran-jamie commented Dec 26, 2023

PR Overview

This PR will address the following Issue/Feature:
fivetran/dbt_zendesk#124

This PR will result in the following new package version:

v0.14.0

Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:

for now, i have a very very brief entry that points to the README

dbt_zendesk_source v0.11.0

🎉 Feature Update 🎉

This release supports running the package on multiple Zendesk sources at once! See the README for details on how to leverage this feature (PR #44).

PR Checklist

Basic Validation

Please acknowledge that you have successfully performed the following commands locally:

  • dbt run –full-refresh && dbt test
  • dbt run (if incremental models are present)

Before marking this PR as "ready for review" the following have been applied:

  • The appropriate issue has been linked, tagged, and properly assigned
  • All necessary documentation and version upgrades have been applied
  • docs were regenerated (unless this PR does not include any code or yml updates) -- waiting on approval
  • BuildKite integration tests are passing
  • Detailed validation steps have been provided below

Detailed Validation

Please share any and all of your validation steps:

See hex notebook linked in Height

Copy link
Contributor

@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.

These changes look great to me! I have a few small comments below, but otherwise this is good to generate the docs and be approved!

As discussed during standup, we will hold off merging and releasing this until all other union data updates are completed this quarter.

README.md Outdated Show resolved Hide resolved
packages.yml Outdated Show resolved Hide resolved
models/stg_zendesk__ticket_field_history.sql Outdated Show resolved Hide resolved
Copy link
Contributor

@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-jamie thanks for working through this PR! I do have a few comments and questions below that I would like for you to take a look at before approving. Let me know if you would like to discuss any of these in more detail. Thanks!

README.md Outdated Show resolved Hide resolved
models/stg_zendesk.yml Show resolved Hide resolved
Copy link
Contributor

@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.

Overall this PR looks great! Just a few comments and suggestions below, but nothing I feel that should block this approval.

packages.yml Outdated Show resolved Hide resolved
models/stg_zendesk__brand.sql Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
Copy link
Contributor

@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.

LGTM

Copy link
Contributor

@fivetran-catfritz fivetran-catfritz left a comment

Choose a reason for hiding this comment

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

@fivetran-jamie Just a few suggestions but otherwise lgtm! Woohoo union data updates!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: fivetran-catfritz <[email protected]>
@fivetran-jamie fivetran-jamie merged commit 94c1b56 into main Dec 2, 2024
7 checks passed
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