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

Change short option for '--webUrl' to '-u' #5478

Closed
Jwaegebaert opened this issue Sep 12, 2023 · 9 comments
Closed

Change short option for '--webUrl' to '-u' #5478

Jwaegebaert opened this issue Sep 12, 2023 · 9 comments

Comments

@Jwaegebaert
Copy link
Contributor

Currently, a few commands within the codebase utilize the short option -w for --webUrl. In contrast, there are +150 commands that use the short -u instead.

To enhance consistency and clarity, I propose changing the short option for --webUrl from -w to -u for all the commands.

@milanholemans
Copy link
Contributor

Let's also not forget to recheck the sample scripts with this breaking change.

@SmitaNachan
Copy link
Contributor

Can I work on this?

@milanholemans
Copy link
Contributor

Yes, definitely!

@SmitaNachan
Copy link
Contributor

Hi @milanholemans, @Jwaegebaert

Below commands already uses the short option -u for --url or --fileUrl options:

In the above commands,

  1. Should we just remove the short option -w from --webUrl or
  2. Shift the -u short option from other options to the --webUrl?

Please suggest.

@milanholemans
Copy link
Contributor

Hi @SmitaNachan great catch! I think consistency is key here, let's take option 2 and consistently use -u for webUrl.
If you can come up with a relevant short for the old option, you may assign a new short to this option. If no relevant short is possible, the old option will no longer have a short, that's no problem.

It's always easier in the future to add short options than to remove them.

@waldekmastykarz
Copy link
Member

Hey @SmitaNachan, will you be able to submit a PR with this change in the next day or two? Totally fine if you can't and would like us to pick it up instead. We've got v7 release coming up and would like to include this change.

@SmitaNachan
Copy link
Contributor

Hi @waldekmastykarz
Yes, I will do it by thursday

@waldekmastykarz
Copy link
Member

Awesome! Thank you so much!

@SmitaNachan
Copy link
Contributor

Hi @waldekmastykarz
PR #5505 is raised. Please review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants