-
Notifications
You must be signed in to change notification settings - Fork 39
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
bugfix/variable-configs #137
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.
looks and runs great! just one question/suggestion in the changelog
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.
Have a couple suggestions, but generally looks good!
Co-authored-by: fivetran-catfritz <111930712+fivetran-catfritz@users.noreply.github.com>
Co-authored-by: fivetran-catfritz <111930712+fivetran-catfritz@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.
lgtm!
PR Overview
This PR will address the following Issue/Feature: Internally raised issue
This PR will result in the following new package version:
v0.17.0
I went back and forth on this one. However, I feel a breaking change is the best path forward here. There is the chance that the sudden introduction of these new variable configs could cause users who have working projects to all of a sudden see missing data. Therefore, I felt it would be best to categorize this as a breaking change to be safe.
Please detail what change(s) this PR introduces and any additional information that should be known during the review of this PR:
🚨 Breaking Changes: Variable Bug Fixes 🚨
hubspot_email_event_sent_enabled
variable:hubspot__contact_lists
email metric fields are now dependent on thehubspot_email_event_sent_enabled
variable being defined astrue
.hubspot__contact
email metric fields are now dependent on thehubspot_email_event_sent_enabled
variable being defined astrue
.hubspot__email_campaigns
model is now dependent on thehubspot_email_event_sent_enabled
variable being defined astrue
.int_hubspot__email_metrics__by_contact_list
model is now dependent on thehubspot_email_event_sent_enabled
variable being defined astrue
.PR Checklist
Basic Validation
Please acknowledge that you have successfully performed the following commands locally:
Before marking this PR as "ready for review" the following have been applied:
Detailed Validation
Please share any and all of your validation steps:
See below for the compilation validation steps:
hubspot_email_event_enabled
defined astrue
and thehubspot_email_event_sent_enabled
variable defined asfalse
I see the following error:If you had to summarize this PR in an emoji, which would it be?
📧