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.

Sign up for GitHub

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

Jump to bottom

Adding a generate_alias_name macro #1363

Merged
merged 9 commits into from
Mar 27, 2019
Merged

Adding a generate_alias_name macro #1363

merged 9 commits into from
Mar 27, 2019

Conversation

nydnarb
Copy link
Contributor

@nydnarb nydnarb commented Mar 21, 2019

This is a draft PR. More details coming soon.

@nydnarb nydnarb marked this pull request as ready for review March 25, 2019 21:24
drewbanin
drewbanin approved these changes Mar 26, 2019
View reviewed changes
Copy link
Contributor

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

Some quick comments, but this looks really great overall! Nice work 🎉

core/dbt/parser/base.py Outdated Show resolved Hide resolved
core/dbt/include/global_project/macros/etc/get_custom_alias.sql Outdated Show resolved Hide resolved
test/integration/043_custom_aliases_test/models/schema.yml Show resolved Hide resolved
nydnarb
nydnarb commented Mar 26, 2019
View reviewed changes
core/dbt/parser/base.py Outdated Show resolved Hide resolved
@nydnarb
Copy link
Contributor Author

nydnarb commented Mar 26, 2019

I think this is good to go! Despite the failing tests. Let me know if there are next steps.

@drewbanin
Copy link
Contributor

ok, thanks @nydnarb! Just kicked off new tests that should pick up our CI credentials. I think we'll be ready to merge this when the tests pass :)

@drewbanin drewbanin added this to the Wilt Chamberlain milestone Mar 26, 2019
@beckjake
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@beckjake beckjake changed the base branch from dev/stephen-girard to dev/wilt-chamberlain March 26, 2019 19:04
@beckjake beckjake changed the base branch from dev/wilt-chamberlain to dev/stephen-girard March 26, 2019 19:05
@beckjake
Copy link
Contributor

@nydnarb I think you have based your call to generate_macro on dev/wilt-chamberlain's behavior, but you are based on dev/stephen-girard. If you rebase onto dev/wilt-chamberlain and change the PR's target branch, I think that failing postgres test will pass.

@nydnarb nydnarb changed the base branch from dev/stephen-girard to dev/wilt-chamberlain March 26, 2019 20:33
@beckjake
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@beckjake beckjake merged commit fcb97bf into dbt-labs:dev/wilt-chamberlain Mar 27, 2019
@beckjake
Copy link
Contributor

Thanks for the contribution @nydnarb!

@nydnarb
Copy link
Contributor Author

nydnarb commented Mar 27, 2019 •
edited
Loading

Thanks @beckjake and @drewbanin. It was fun contributing! Will this feature be included on 0.13.1 or is this something that will wait until 0.14.0?

@beckjake beckjake mentioned this pull request May 2, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drewbanin drewbanin drewbanin approved these changes

@beckjake beckjake beckjake left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Wilt Chamberlain
Development

Successfully merging this pull request may close these issues.
3 participants
@nydnarb @drewbanin @beckjake