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

Handle warnings from dbt ls commands (Issue #12) #15

Merged
merged 3 commits into from
Jan 3, 2022
Merged

Handle warnings from dbt ls commands (Issue #12) #15

merged 3 commits into from
Jan 3, 2022

Conversation

robastel
Copy link
Member

@robastel robastel commented Jan 1, 2022

Closes Issue #12

@robastel
Handle warnings from dbt ls commands
e26eaa8 
Signed-off-by: Robert Astel <rob.astel@gmail.com>
@robastel
Reformat with Black
672067d 
Signed-off-by: Robert Astel <rob.astel@gmail.com>
@robastel robastel self-assigned this Jan 1, 2022
@robastel robastel linked an issue Jan 1, 2022 that may be closed by this pull request
Bug: properties command fails when dbt issues warning #12
Closed
@jenniferczhan
Copy link

Had trouble commenting on this line bc it wasn't changed, but I was wondering if it makes more sense for the default to be 'json' instead of 'path' going forward if --output json will contain the file path in future versions of dbt?

jenniferczhan
jenniferczhan previously approved these changes Jan 2, 2022
View reviewed changes
Copy link

@jenniferczhan jenniferczhan left a comment

Choose a reason for hiding this comment

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

Left some comments and clarifying questions, but the main changes make sense to me.

@robastel
Copy link
Member Author

robastel commented Jan 2, 2022

Had trouble commenting on this line bc it wasn't changed, but I was wondering if it makes more sense for the default to be 'json' instead of 'path' going forward if --output json will contain the file path in future versions of dbt?

I'll make the changes to take into account this suggestion.

@robastel
Make json default for dbt ls, do not rerun dbt ls unnecessarily
aeff664 
Signed-off-by: Robert Astel <rob.astel@gmail.com>
jenniferczhan
jenniferczhan approved these changes Jan 3, 2022
View reviewed changes
Copy link

@jenniferczhan jenniferczhan left a comment

Choose a reason for hiding this comment

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

Changes make sense!

@robastel robastel merged commit 949c0eb into main Jan 3, 2022
@robastel robastel deleted the issue-12 branch January 3, 2022 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jenniferczhan jenniferczhan jenniferczhan approved these changes

Assignees

@robastel robastel

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: properties command fails when dbt issues warning
2 participants
@robastel @jenniferczhan