Skip to content
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

Change tabs in dbt_project.yml to 2 spaces instead of 4 #11

Merged
merged 1 commit into from
Mar 15, 2021

Conversation

alexiswo
Copy link
Contributor

Details

Changes the dbt_project.yml file tab length to align with the tab length of 2 spaces specified in the YAML style guide section of the internal dbt coding convention docs AND the dbt_project.yml page on docs. Unifying the tab length will make it easier for users to copy and paste from the docs into their first projects, an issue that we see a lot of users run into during dbt Learn sessions.

Testing

Copy and pasted this into the dbt_project.yml in a new project in dbt Cloud. Ran dbt run without any issues. See screenshots below.

Screen Shot 2021-01-27 at 3 07 01 PM

Screen Shot 2021-01-27 at 3 07 15 PM

@alexiswo
Copy link
Contributor Author

@drewbanin - would love if you could test this before merging as I'm worried about breaking everyone's dbt_project.yml files!

@drewbanin
Copy link
Contributor

thanks @alexiswo ! I didn't think of this at first, but there's one thing to note here: The dbt init command created a project by cloning this tag. So, just merging this PR isn't actually going to update dbt's behavior. We're either going to need to update the tag (need to think on that one) or change how dbt works to not point to this one specific tag (maybe a branch instead?).

@jtcohen6 i feel like we discussed this recently - do you recall where we ended up on this question?

@jtcohen6
Copy link

Where we ended up: We should start vendoring the starter project through dbt directly, rather than having it live in this repo separately (dbt-labs/dbt-core#3005)

@alexiswo
Copy link
Contributor Author

alexiswo commented Jan 28, 2021

Got it. So, should I leave this PR here? Or put a ticket somewhere for the future when dbt-labs/dbt-core#3005 is done?

@jtcohen6
Copy link

I think we can move ahead with this PR as is. That way, when we get around to dbt-labs/dbt-core#3005, we'll be moving the right (updated) code into the dbt repo :)

@drewbanin drewbanin requested review from jtcohen6 and removed request for drewbanin January 29, 2021 12:31
Copy link

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

Good by me!

Note that, because dbt init on versions >0.17.0 clones from the dbt-yml-config-version-2 tag, we'd need to recreate the dbt-yml-config-version-2 tag for this to take effect in newer vresions, which is a bit tricky. The whole thing is trickier than it needs to be IMO; I'd much rather just move this code into the dbt repo and have starter projects tied to particular versions (dbt-labs/dbt-core#3005).

Also, this is (effectively) the same as #8, so I think we can close that after merging this. (There's a question there about whether to include the + in front of the materialized config, but let's save that for another day.)

@alexiswo
Copy link
Contributor Author

Whoops! Didn't realize @jtcohen6 approved this. Merging now

@alexiswo alexiswo merged commit e5e65a1 into master Mar 15, 2021
@alexiswo alexiswo deleted the alexis-yml-tab-length branch March 15, 2021 21:02
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