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

Support token and url keys without hyphens #70

Merged
merged 7 commits into from
Aug 4, 2023

Conversation

joshuatcasey
Copy link
Contributor

Summary

Support token and url keys without hyphens

Use Cases

CF Java buildpack uses apitoken and apiurl instead of api-token and api-url.

This PR will allow using either option, preferring the existing api-token and api-url options.

https://paketobuildpacks.slack.com/archives/C040DPFJMQV/p1689078089284269

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

@joshuatcasey joshuatcasey requested a review from a team as a code owner July 12, 2023 20:13
@dmikusa dmikusa added semver:minor A change requiring a minor version bump type:enhancement A general enhancement labels Jul 17, 2023
@dmikusa
Copy link
Contributor

dmikusa commented Jul 17, 2023

/easycla

README.md Outdated
| `api-token` | (Required) The token for communicating with the Dynatrace service.
| Key | Value Description |
|------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `api-url`, `apiurl`, or `environment-id` | (Required) The base URL of the Dynatrace API. If not set, `environment-id` must be set. <br/> --- <br/> If neither `api-url` or `apiurl` is set, a URL is configured in the form: https://<`environment-id`>.live.dynatrace.com/api. |
Copy link
Contributor

Choose a reason for hiding this comment

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

I worry that listing both might create some consternation with users over which one to use (it could just be my brain also). Anyway, while this PR makes it technically accept either one, I think we should continue to promote using api-url in this section (i.e. no need to change this table). It's just more direct that way.

I think your **Note** at the bottom is sufficient to document that fact that we also permit the keys without dashes. The key is that for compatibility we allow it to do both. If you need it to not have the - then it'll work, but we recommend using it with the -.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just pushed a change - please review and resolve if that works for you.

@dmikusa
Copy link
Contributor

dmikusa commented Jul 17, 2023

Looks good, just two minor notes.

@joshuatcasey joshuatcasey requested a review from dmikusa July 19, 2023 19:37
@dmikusa dmikusa merged commit 1c47e07 into main Aug 4, 2023
@dmikusa dmikusa deleted the jtc/support-keys-without-hyphens branch August 4, 2023 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor A change requiring a minor version bump type:enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants