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

clarify use cases for packages #4410

Merged
merged 14 commits into from
Nov 29, 2023
Merged

clarify use cases for packages #4410

merged 14 commits into from
Nov 29, 2023

Conversation

mirnawong1
Copy link
Contributor

clarifying the use cases for dpeendencies.yml and packages.yml per slack convo in dbt slack community.

@mirnawong1 mirnawong1 requested a review from a team as a code owner November 7, 2023 16:03
@vercel Vercel
Copy link

vercel bot commented Nov 7, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs-getdbt-com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 29, 2023 10:40am

@github-actions github-actions bot added content Improvements or additions to content Docs team Authored by the Docs team @dbt Labs size: medium This change will take up to a week to address labels Nov 7, 2023
@VersusFacit
Copy link
Contributor

VersusFacit commented Nov 7, 2023 •
edited
Loading

I'm gonna offer a soft approval here. I really think you should wait for Jeremy to sign off on the accuracy.

@mirnawong1
Copy link
Contributor Author

hey @jtcohen6 , whenever you have a chance -- could you provide a technical review this this pr which clarifies the packages.yml/depencies.yml use case based on that community thread between Mila and us? thank you!

@mirnawong1
Copy link
Contributor Author

followed up for tech review

jtcohen6
jtcohen6 reviewed Nov 27, 2023
View reviewed changes
Copy link
Collaborator

@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.

@mirnawong1 Sorry this took me so long. Let's talk a bit more about which points really need clarifying / emphasizing here.

website/snippets/_packages_or_dependencies.md Outdated Show resolved Hide resolved
website/snippets/_packages_or_dependencies.md Show resolved Hide resolved
@mirnawong1
Copy link
Contributor Author

hey @jtcohen6 - thanks for that feedback and made some changes to fold them in/address them. can you have another look when you have a moment pls? thank you!

jtcohen6
jtcohen6 approved these changes Nov 29, 2023
View reviewed changes
Copy link
Collaborator

@jtcohen6 jtcohen6 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you @mirnawong1! This gets at the message that I want us to be communicating

@mirnawong1 mirnawong1 enabled auto-merge November 29, 2023 10:35
@mirnawong1
Copy link
Contributor Author

woohoo! amazing and thanks @jtcohen6 ! going to merge this now!

@mirnawong1 mirnawong1 merged commit 615c24e into current Nov 29, 2023
6 checks passed
@mirnawong1 mirnawong1 deleted the mwong-clarify-use-case branch November 29, 2023 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VersusFacit VersusFacit VersusFacit left review comments

@jtcohen6 jtcohen6 jtcohen6 approved these changes

Assignees
No one assigned
Labels
content Improvements or additions to content Docs team Authored by the Docs team @dbt Labs size: medium This change will take up to a week to address
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mirnawong1 @VersusFacit @jtcohen6