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

Implement --version in click #6760

Closed
wants to merge 15 commits into from
Closed

Implement --version in click #6760

wants to merge 15 commits into from

Conversation

stu-k
Copy link
Contributor

@stu-k stu-k commented Jan 26, 2023

resolves #6757

Checklist

@cla-bot cla-bot bot added the cla:yes label Jan 26, 2023
@stu-k stu-k closed this Jan 26, 2023
@stu-k stu-k reopened this Jan 26, 2023
@stu-k stu-k closed this Jan 27, 2023
@stu-k stu-k reopened this Jan 27, 2023
@stu-k stu-k closed this Jan 27, 2023
@stu-k stu-k reopened this Jan 27, 2023
@stu-k stu-k force-pushed the CT-1926/migrate-version-task branch from faea954 to 1404177 Compare January 30, 2023 16:37
@stu-k stu-k closed this Jan 30, 2023
@stu-k stu-k reopened this Jan 30, 2023
@stu-k stu-k marked this pull request as ready for review January 30, 2023 18:27
@stu-k stu-k requested review from leahwicz and a team as code owners January 30, 2023 18:27
@stu-k stu-k requested a review from Fleid January 30, 2023 18:27
stu-k
stu-k commented Jan 30, 2023
View reviewed changes
core/dbt/cli/params.py Outdated Show resolved Hide resolved
stu-k
stu-k commented Jan 30, 2023
View reviewed changes
tests/unit/test_dbt_runner.py Outdated Show resolved Hide resolved
@stu-k stu-k requested review from iknox-fa, MichelleArk, aranke, ChenyuLInx and a team and removed request for Fleid and leahwicz January 30, 2023 18:32
aranke
aranke reviewed Jan 30, 2023
View reviewed changes
Copy link
Member

@aranke aranke left a comment

Choose a reason for hiding this comment

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

Can we use the @click.version_option(...) decorator as described here?

https://stackoverflow.com/a/58666832

@stu-k stu-k requested a review from aranke January 30, 2023 19:42
@stu-k stu-k closed this Jan 30, 2023
@stu-k stu-k reopened this Jan 30, 2023
iknox-fa
iknox-fa approved these changes Jan 30, 2023
View reviewed changes
Copy link
Contributor

@iknox-fa iknox-fa left a comment

Choose a reason for hiding this comment

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

LGTM

aranke
aranke reviewed Jan 30, 2023
View reviewed changes
tests/unit/test_dbt_runner.py Outdated Show resolved Hide resolved
core/dbt/cli/main.py Outdated Show resolved Hide resolved
@iknox-fa
Copy link
Contributor

FYI -- This PR will have to be merged before the tests will pass because we actually use the version command in our CI pipeline.

@stu-k stu-k closed this Jan 31, 2023
@stu-k stu-k reopened this Jan 31, 2023
@stu-k stu-k requested a review from a team as a code owner January 31, 2023 17:17
@stu-k stu-k requested a review from emmyoop January 31, 2023 17:17
@stu-k stu-k removed request for a team and emmyoop January 31, 2023 17:24
ChenyuLInx
ChenyuLInx reviewed Jan 31, 2023
View reviewed changes
Copy link
Contributor

@ChenyuLInx ChenyuLInx left a comment

Choose a reason for hiding this comment

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

Changes looks good, although it seems also contain a few other changes that haven't been merged yet? maybe we should wait for those to merge before merging this?

@stu-k
Copy link
Contributor Author

stu-k commented Jan 31, 2023

@ChenyuLInx yeah I messed something up when trying to merge feature/click-cli into this branch. Give me a moment to fix it 🤦

@stu-k stu-k mentioned this pull request Jan 31, 2023
Merged
6 tasks
@stu-k
Copy link
Contributor Author

stu-k commented Jan 31, 2023

Closing in favor of #6802

@stu-k stu-k closed this Jan 31, 2023
@stu-k stu-k deleted the CT-1926/migrate-version-task branch February 20, 2023 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChenyuLInx ChenyuLInx ChenyuLInx left review comments

@aranke aranke aranke left review comments

@iknox-fa iknox-fa iknox-fa approved these changes

@MichelleArk MichelleArk Awaiting requested review from MichelleArk

Assignees
No one assigned
Labels
cla:yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@stu-k @iknox-fa @MichelleArk @ChenyuLInx @aranke @FishtownBuildBot