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

Profile works w Click #6336

Merged
merged 6 commits into from
Dec 8, 2022

Conversation

iknox-fa
Copy link
Contributor

@iknox-fa iknox-fa commented Nov 29, 2022

resolves #5536

Description

Allows the new CLick cli to access Profiles to pass in to various subcommands/tasks.
Note: This PR breaks functional tests (new framework). The work to update that test framework is being handled in another ticket

Checklist

@cla-bot cla-bot bot added the cla:yes label Nov 29, 2022
@github-actions
Copy link
Contributor

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

@iknox-fa iknox-fa force-pushed the iknox/CT-912-GH-5536-Profile-for-click branch from 448e401 to 9949d99 Compare November 29, 2022 18:20
@iknox-fa iknox-fa changed the title support lower cased flags for legacy args compat Profile works w Click Nov 29, 2022
@ChenyuLInx ChenyuLInx mentioned this pull request Dec 2, 2022
6 tasks
@iknox-fa iknox-fa force-pushed the iknox/CT-912-GH-5536-Profile-for-click branch from d4bd1eb to ae77041 Compare December 4, 2022 21:32
@iknox-fa iknox-fa changed the base branch from feature/click-cli to main December 4, 2022 22:14
@iknox-fa iknox-fa changed the base branch from main to feature/click-cli December 4, 2022 22:14
@iknox-fa iknox-fa marked this pull request as ready for review December 4, 2022 22:19
@iknox-fa iknox-fa requested a review from a team December 4, 2022 22:19
@iknox-fa iknox-fa requested review from a team as code owners December 4, 2022 22:19
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.

LGTM! Thanks for porting all of the changes together!

Copy link
Contributor

@stu-k stu-k left a comment

Choose a reason for hiding this comment

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

Nothing stands out 👍

Moving all logic regarding constructing a complete UnsetProfile to this function
This way we can have a clean load_profile function to call by the new CLI and remove
all logic for UnsetProfile once we migrate to new click CLI
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@ChenyuLInx
Copy link
Contributor

Just realized one thing, deps command fail now, since it doesn't require some attributes in flag that are being accessed incli function now

@iknox-fa iknox-fa merged commit 88d2ee4 into feature/click-cli Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants