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

Fix examples for dbt deps --add-package #4470

Merged
merged 7 commits into from
Nov 15, 2023

Conversation

dbeatty10
Copy link
Contributor

@dbeatty10 dbeatty10 commented Nov 15, 2023

Preview

What are you changing in this pull request and why?

Fixing the code examples per dbt-labs/dbt-core#9076 by using test examples from dbt-labs/dbt-core#8408

Checklist

  • Review the Content style guide and About versioning so my content adheres to these guidelines.
  • I've checked that the code examples work
  • I've confirmed that the preview renders correctly

Copy link

vercel bot commented Nov 15, 2023

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 15, 2023 9:40pm

@github-actions github-actions bot added content Improvements or additions to content size: small This change will take 1 to 2 days to address labels Nov 15, 2023
@dbeatty10 dbeatty10 marked this pull request as ready for review November 15, 2023 00:32
@dbeatty10 dbeatty10 requested a review from a team as a code owner November 15, 2023 00:32
@dbeatty10 dbeatty10 enabled auto-merge (squash) November 15, 2023 00:32
@seub
Copy link

seub commented Nov 15, 2023

Thank you for creating this PR @dbeatty10.

While you're on this, the doc mentions:

You can run dbt deps --lock to update the package-lock.yml with the most recent dependencies from packages

I could be wrong, but by glancing at the code, it seems to me that this is inaccurate. The correct statement should be along the lines of:

You can pass the flags:
--lock: Create or replace the lock file if it does not exist or if packages.yml has changed, and stop (do not install the packages).
--upgrade: Create or replace the lock file with the latest compatible versions, and install the packages unless the flag --lock is also passed.

Note: I actually like this "actual behavior" better than the behavior implied by the documentation. However, I don't see the need for both the flags --dry-run and --lock, they could be merged into a same flag as far as I can tell.

There is also a typo "depenedencies.yml" on the page.

Copy link
Contributor

@matthewshaver matthewshaver left a comment

Choose a reason for hiding this comment

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

Thank you @dbeatty10

@dbeatty10 dbeatty10 merged commit ac5ad7b into current Nov 15, 2023
5 checks passed
@dbeatty10 dbeatty10 deleted the dbeatty/fix-dbt-deps-add-package branch November 15, 2023 21:40
@dbeatty10
Copy link
Contributor Author

@seub thanks for taking a look at this PR and making these suggestions 🏆

@mirnawong1 fixed the spelling of "dependencies" within 5e17f75.

I've opened this issue to consider your other suggestions: #4481

@dbeatty10
Copy link
Contributor Author

@seub also opened this feature request as well: dbt-labs/dbt-core#9100

@seub
Copy link

seub commented Nov 16, 2023

Thank you @dbeatty10 ! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content Improvements or additions to content size: small This change will take 1 to 2 days to address
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants