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

feat(cli): add alias flags for swa cli commands and options #453

Merged
merged 6 commits into from
May 17, 2022
Merged

feat(cli): add alias flags for swa cli commands and options #453

merged 6 commits into from
May 17, 2022

Conversation

Devansu-Yadav
Copy link
Contributor

@Devansu-Yadav Devansu-Yadav commented Apr 25, 2022 •
edited
Loading

Introduce alias flags for CLI config options specified by user. This PR resolves #410

Alias flags and Unit tests added for:-

@github-actions github-actions bot added the scope: cli Issues happened a the ./src/cli level label Apr 25, 2022
@Devansu-Yadav Devansu-Yadav marked this pull request as draft April 25, 2022 08:36
@Devansu-Yadav
Copy link
Contributor Author

@manekinekko Please review some of the tests that I have added in https://github.com/Azure/static-web-apps-cli/tree/ga/src/core/utils/options.spec.ts file as per your convenience so that I know if I'm doing it correctly or not 😅

@manekinekko
Copy link
Member

You are in the right direction 👍

@manekinekko manekinekko added the scope: core Issues happened a the ./src/core level label Apr 25, 2022
@github-actions github-actions bot removed the scope: core Issues happened a the ./src/core level label Apr 27, 2022
@Devansu-Yadav Devansu-Yadav force-pushed the addAliasFlags branch 2 times, most recently from 6a86e07 to 01fb025 Compare April 30, 2022 15:10
@Devansu-Yadav
Copy link
Contributor Author

Devansu-Yadav commented May 1, 2022 •
edited
Loading

@manekinekko @sinedied I have finished adding alias flags for all of the commands that have been created so far along with their corresponding Unit tests. Please review the alias flags for the command options and do let me know if they need to be updated accordingly.

P.S:- I haven't added alias flags for a few short options like --env, --auto since I thought that it might not be needed as they are short (Do let me know, if needed, I can add them).

@manekinekko
Copy link
Member

@Devansu-Yadav thank you for taking the time to make those changes. Would you mind rebasing your PR on the ga branch one last time? The GA branch is ready, and ee are planning on merging this PR by tomorrow 😊

@Devansu-Yadav
Copy link
Contributor Author

@Devansu-Yadav thank you for taking the time to make those changes. Would you mind rebasing your PR on the ga branch one last time? The GA branch is ready, and ee are planning on merging this PR by tomorrow 😊

Sure, I'll make the changes! 👍

@Devansu-Yadav
Copy link
Contributor Author

@manekinekko I have rebased and added alias flags for some of the new options that were added on the ga branch. Also updated README.md with these new alias flags for existing commands (I haven't added the alias flags for commands like login since it hasn't been added to docs yet 😅).

@Devansu-Yadav Devansu-Yadav marked this pull request as ready for review May 16, 2022 16:46
sinedied
sinedied approved these changes May 17, 2022
View reviewed changes
Copy link
Member

@sinedied sinedied left a comment

Choose a reason for hiding this comment

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

I've made some tiny changes as it seems some recent flags changes were lost in your rebase, thanks for your PR!

@sinedied sinedied merged commit 7e09e0d into Azure:ga May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sinedied sinedied sinedied approved these changes

@manekinekko manekinekko manekinekko approved these changes

Assignees

@Devansu-Yadav Devansu-Yadav

Labels
scope: cli Issues happened a the ./src/cli level
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Devansu-Yadav @manekinekko @sinedied