-
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
Addressing the FR raised against the package to add the title field t… #17
Addressing the FR raised against the package to add the title field t… #17
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.
This looks really good! Largely left Hubspot suggestions/comments around using different sorts of titles.
FYI I pushed a commit to your branch fixing an unrelated issue around the rag__using_<zendesk/jira/hubspot>
variables, so you'll need to git fetch && git pull origin feature/add-title-to-rag__unified_document
before making any new changes
Thank you so much for helping us out here @fivetran-poonamagate !
models/intermediate/hubspot/int_rag_hubspot__deal_comment_document.sql
Outdated
Show resolved
Hide resolved
models/intermediate/hubspot/int_rag_hubspot__deal_comment_document.sql
Outdated
Show resolved
Hide resolved
models/intermediate/hubspot/int_rag_hubspot__deal_comment_document.sql
Outdated
Show resolved
Hide resolved
models/intermediate/hubspot/int_rag_hubspot__deal_comment_document.sql
Outdated
Show resolved
Hide resolved
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.
This looks awesome @fivetran-poonamagate! Couple of super tiny suggestions. If you have time to address them, that'd be great, but the PR is already in a great spot for our team to take over for polishes and such if not
models/intermediate/hubspot/int_rag_hubspot__deal_comment_document.sql
Outdated
Show resolved
Hide resolved
models/intermediate/hubspot/int_rag_hubspot__deal_comment_document.sql
Outdated
Show resolved
Hide resolved
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.
A few change requests before approval
Co-authored-by: Joe Markiewicz <[email protected]>
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.
LGTM
…o relevant models
Please provide your name and company
Poonam Agate
Link the issue/feature request which this PR is meant to address
#15
Detail what changes this PR introduces and how this addresses the issue/feature request linked above.
Adds the title field to the rag__unified_document model
How did you validate the changes introduced within this PR?
Ran the proposed package updates in BigQuery and confirmed that the models ran successfully
Which warehouse did you use to develop these changes?
BigQuery
Did you update the CHANGELOG?
Did you update the dbt_project.yml files with the version upgrade (please leverage standard semantic versioning)? (In both your main project and integration_tests)
Typically there are additional maintenance changes required before this will be ready for an upcoming release. Are you comfortable with the Fivetran team making a few commits directly to your branch?
If you had to summarize this PR in an emoji, which would it be?
🐝
Feedback
We are so excited you decided to contribute to the Fivetran community dbt package! We continue to work to improve the packages and would greatly appreciate your feedback on our existing dbt packages or what you'd like to see next.
PR Template
Community Pull Request Template (default)
Maintainer Pull Request Template (to be used by maintainers)