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

Adding -H (Header) as a global option #115

Merged
merged 5 commits into from
Apr 23, 2018
Merged

Adding -H (Header) as a global option #115

merged 5 commits into from
Apr 23, 2018

Conversation

kjshomper
Copy link
Contributor

Our team would like to make use of the apigeetool; however, we're behind a company proxy and need to provide authorization to get out to the internet and hit the api.enterprise.apigee.com endpoint. CNTLM is an option for local development environments, but doesn't work well in CI pipelines.

I added two changes:

  1. Added a global -H header option to provide additional headers from the command line.

  2. Extended the option descriptors to allow multiples and updated the option parsing code to push multiple option values onto an array. I did not alter defaults.js to consume multiples on any options other than header. I left that as a change awaiting a use case that needed multiples on other options.

So these changes should be fully compatible with any prior use, but does extend capability for those who need to set headers.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@kjshomper
Copy link
Contributor Author

kjshomper commented Apr 19, 2018 via email

@googlebot
Copy link

CLAs look good, thanks!

@kjshomper
Copy link
Contributor Author

Added a fix. It appears the intent of the help output was to show required when the option must be included on the command line. Although this wasn't the behavior.

Before (fetchproxy -help example) , if the option was required and the program prompted for it, it would show as required, which is was, but not on the command line. Other required options, not prompted for, like API or Organization, were listed in the help as optional. This is confusing, because supplying only username and password still resulted in an error message identifying the required options.

After: if the option is required and not prompted for (false) or prompt is undefined, then the program will not obtain the value and should fail, therefore, the options should be listed as required (for the command line). If the option is required and prompted for, i.e. prompt: true, then it's still optional on the command line,, because it will prompt. If the option isn't required, it's optional. Changing the test on prompt to a NOT test, fixes the behavior.

@AdamMagaluk
Copy link
Contributor

LGTM except can you add a note in the READ.md for the flag. It should go in the Common Parameters section.

Thanks

@kjshomper
Copy link
Contributor Author

Made the update

@AdamMagaluk
Copy link
Contributor

Looks good just need to remove .vscode/launch.json file.

@kjshomper
Copy link
Contributor Author

Done. Didn't think on the git status and used the git add . by mistake.

@kjshomper
Copy link
Contributor Author

Thanks for checking.

@AdamMagaluk
Copy link
Contributor

LGTM, thanks for the addition!

@AdamMagaluk AdamMagaluk merged commit 559e416 into apigee:master Apr 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants