Skip to content

Add conversions to insights streams #204

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

Merged
merged 3 commits into from
Aug 16, 2023
Merged

Conversation

GtheSheep
Copy link
Contributor

Description of change

Include schemas for conversions in insights tables
Remove duplicate "reach" in schema of ads_insights

Manual QA steps

  • Running this tap daily and comparing to values in Meta ads insights manager

Risks

  • Any unexpected formats of conversions on various API versions? But can't see any obvious changes

Rollback steps

  • revert this branch

@singer-bot
Copy link

Hi @GtheSheep, thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

@singer-bot
Copy link

You did it @GtheSheep!

Thank you for signing the Singer Contribution License Agreement.

@beauchbum
Copy link

Hi @GtheSheep, checking if this is still in progress? Looking forward to this getting merged so we can leverage conversions data. Thanks!

@GtheSheep
Copy link
Contributor Author

Hey @beauchbum - I've been running this off my fork for a couple of months now and all seems well, not sure what else is required to get this merged other than for someone to review?

@GtheSheep
Copy link
Contributor Author

@dsprayberry @luandy64 @kethan1122 - not sure what the process is to get this reviewed, any insight plz?

@sgandhi1311
Copy link
Member

@GtheSheep thanks for your contribution. I will review the change and get it deployed.

@sgandhi1311 sgandhi1311 merged commit 8cef617 into singer-io:master Aug 16, 2023
@sgandhi1311
Copy link
Member

@GtheSheep @beauchbum, the PR changes have been deployed to production. Could you verify on your end? Thank you!

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.

4 participants