Skip to content

Comments

PPC Analysis#16

Open
henryhund wants to merge 8 commits intoanalyst-collective:masterfrom
henryhund:ppc-analysis
Open

PPC Analysis#16
henryhund wants to merge 8 commits intoanalyst-collective:masterfrom
henryhund:ppc-analysis

Conversation

@henryhund
Copy link

No description provided.

@@ -1,4 +0,0 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

I agree that config.json probably shouldn't be checked in, but maybe we can check in a file like config.sample.json w/ example parameters?

@drewbanin
Copy link
Member

this looks great @henryhund. Keen to hear thoughts form @jthandy and @cmerrick

@@ -0,0 +1,35 @@
create or replace view ac_hhund.facebook_ads_summary as (
Copy link
Member

Choose a reason for hiding this comment

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

replace ac_hhund with {schema}

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@jthandy
Copy link
Member

jthandy commented Mar 16, 2016

Two tiny comments otherwise I'm 👍

@henryhund
Copy link
Author

@jthandy refactored this to set up ppc as a model, rather than analyses. per our convo yesterday.

@jthandy
Copy link
Member

jthandy commented Mar 17, 2016

This is a bit behind dbt at the moment; you really need to update these models to use current dbt conventions or it will cause builds from master to fail. Please LMK if you need help setting up dbt or with how to change your models in order to conform.

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