Feature/new union macro metadata fields and variable configs#140
Conversation
fivetran-joemarkiewicz
left a comment
There was a problem hiding this comment.
@fivetran-savage great work on this PR! A few comments before approving. Let me know if you have any questions, thanks!
integration_tests/tests/consistency/consistency_invoice_line_item_details.sql
Outdated
Show resolved
Hide resolved
integration_tests/tests/consistency/consistency_subscription_details.sql
Outdated
Show resolved
Hide resolved
fivetran-joemarkiewicz
left a comment
There was a problem hiding this comment.
@fivetran-savage this PR is looking good and close to approval! A few more comments before approval
integration_tests/tests/consistency/consistency_invoice_line_item_details.sql
Outdated
Show resolved
Hide resolved
integration_tests/tests/consistency/consistency_invoice_line_item_details.sql
Outdated
Show resolved
Hide resolved
Co-authored-by: Joe Markiewicz <74217849+fivetran-joemarkiewicz@users.noreply.github.com>
Co-authored-by: Joe Markiewicz <74217849+fivetran-joemarkiewicz@users.noreply.github.com>
Co-authored-by: Joe Markiewicz <74217849+fivetran-joemarkiewicz@users.noreply.github.com>
fivetran-joemarkiewicz
left a comment
There was a problem hiding this comment.
LGTM once above suggestions are applied, docs are generated, and integration tests are passing. Great work!
Co-authored-by: Joe Markiewicz <74217849+fivetran-joemarkiewicz@users.noreply.github.com>
fivetran-avinash
left a comment
There was a problem hiding this comment.
@fivetran-savage Looking good, great updates!
A host of minor tweaks and updates, then a few doc requests before approval.
|
|
||
| ), subscription_item as ( | ||
|
|
||
| select * |
There was a problem hiding this comment.
looks like this was one of two ctes that didn't get an indent update so it looks slightly off
Co-authored-by: Avinash Kunnath <108772760+fivetran-avinash@users.noreply.github.com> Co-authored-by: Joe Markiewicz <74217849+fivetran-joemarkiewicz@users.noreply.github.com>
Co-authored-by: Joe Markiewicz <74217849+fivetran-joemarkiewicz@users.noreply.github.com>
Co-authored-by: Avinash Kunnath <108772760+fivetran-avinash@users.noreply.github.com>
Co-authored-by: Avinash Kunnath <108772760+fivetran-avinash@users.noreply.github.com>
fivetran-avinash
left a comment
There was a problem hiding this comment.
@fivetran-savage Some final comments + reverting the indentation updates as discussed before approval.
Co-authored-by: Avinash Kunnath <108772760+fivetran-avinash@users.noreply.github.com>
fivetran-avinash
left a comment
There was a problem hiding this comment.
@fivetran-savage Two small changes but lgtm afterward!
Co-authored-by: Avinash Kunnath <108772760+fivetran-avinash@users.noreply.github.com>
PR Overview
Package version introduced in this PR:
This PR addresses the following Issue/Feature(s):
Summary of changes:
Submission Checklist
Changelog