-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add config for dbt deps #1565
Add config for dbt deps #1565
Conversation
WalkthroughThe pull request introduces a new configuration property Changes
Sequence DiagramsequenceDiagram
participant User
participant Extension
participant DBTProject
participant DependencyManager
User->>Extension: Configure project
Extension->>DBTProject: Initialize project
DBTProject->>Extension: Check installDepsOnProjectInitialization
alt Dependencies should be installed
DBTProject->>DependencyManager: installDeps(silent)
DependencyManager-->>DBTProject: Dependencies installed
else Dependencies installation disabled
DBTProject-->>Extension: Skip dependency installation
end
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to f85b62c in 32 seconds
More details
- Looked at
50
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. src/manifest/dbtProject.ts:661
- Draft comment:
TheinstallDeps
function now accepts asilent
parameter, which is used to set thefocus
property of theinstallDepsCommand
. This change is consistent with the new configuration optiondbt.installDepsOnProjectInitialization
. - Reason this comment was not posted:
Confidence changes required:10%
The functioninstallDeps
has been modified to accept asilent
parameter, which is used to set thefocus
property of theinstallDepsCommand
. This change is consistent with the new configuration optiondbt.installDepsOnProjectInitialization
.
2. src/manifest/dbtProject.ts:661
- Draft comment:
Please specify a return type for theinstallDeps
function to improve code readability and maintainability. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_e0gVQpC5P2hdhyly
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/manifest/dbtProject.ts (1)
661-669
: Add JSDoc documentation for the new parameter.While the implementation is correct, consider adding JSDoc documentation to describe the new
silent
parameter and its purpose.+/** + * Install dbt dependencies. + * @param silent When true, prevents the installation command from focusing on the terminal + * @returns Promise that resolves when dependencies are installed + */ async installDeps(silent = false) { this.telemetry.sendTelemetryEvent("installDeps"); const installDepsCommand = this.dbtCommandFactory.createInstallDepsCommand(); if (silent) { installDepsCommand.focus = false; } return this.dbtProjectIntegration.deps(installDepsCommand); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json
(1 hunks)src/manifest/dbtProject.ts
(2 hunks)
🔇 Additional comments (2)
package.json (1)
124-128
: LGTM! Well-structured configuration property.The new configuration property is well-defined with:
- Clear description
- Appropriate default value
- Consistent naming convention
src/manifest/dbtProject.ts (1)
526-529
: LGTM! Well-implemented configuration check.The implementation correctly:
- Retrieves the configuration with appropriate default
- Guards against multiple initializations using the depsInitialized flag
- Integrates smoothly with the existing rebuildManifest flow
Overview
Problem
Describe the problem you are solving. Mention the ticket/issue if applicable.
Solution
Describe the implemented solution. Add external references if needed.
Screenshot/Demo
A picture is worth a thousand words. Please highlight the changes if applicable.
How to test
Checklist
README.md
updated and added information about my changeImportant
Adds configuration to automatically install dbt dependencies on project initialization, with default enabled, in
DBTProject
class.dbt.installDepsOnProjectInitialization
configuration inpackage.json
to control automatic installation of dbt dependencies during project initialization.true
, meaning dependencies are installed by default.DBTProject
class,rebuildManifest()
method checksinstallDepsOnProjectInitialization
setting and installs dependencies if not initialized.installDeps()
method to accept asilent
parameter to control command focus.This description was created by
for f85b62c. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Configuration
dbt.installDepsOnProjectInitialization
added with default value oftrue