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

Rename Configuration Validating Webhook #316

Conversation

kate-goldenring
Copy link
Contributor

What this PR does / why we need it:
Akri supports a webhook for validating configurations. This is described as "webhook configuration" in our Helm chart and rust binary. A name that described what it does, act as a "configuration validator", would be preferable.

Special notes for your reviewer:
I noticed that that "webhook-configuration" is misleading when renaming Configuration templates in this PR #315.
The name could be interpreted as an Akri Configuration for "discovering" webhooks.

If applicable:

  • this PR contains documentation
  • this PR contains unit tests
  • added code adheres to standard Rust formatting (cargo fmt)
  • code builds properly (cargo build)
  • code is free of common mistakes (cargo clippy)
  • all Akri tests succeed (cargo test)
  • inline documentation builds (cargo doc)
  • version has been updated appropriately (./version.sh)

@kate-goldenring kate-goldenring force-pushed the rename-webhook-configuration branch from 8b1af8c to 81775a2 Compare April 22, 2021 16:10
@kate-goldenring
Copy link
Contributor Author

So after looking further, it looks like webhook-configuration is a fine naming choice in the Kubernetes world, as our is a ValidatingWebhookConfiguration as can also be noted by looking at the Helm template for the webhook. I am now thinking a name change may not be needed or appropriate. @romoh @bfjelds thoughts?

@bfjelds
Copy link
Collaborator

bfjelds commented Apr 22, 2021

So after looking further, it looks like webhook-configuration is a fine naming choice in the Kubernetes world, as our is a ValidatingWebhookConfiguration as can also be noted by looking at the Helm template for the webhook. I am now thinking a name change may not be needed or appropriate. @romoh @bfjelds thoughts?

i suppose it is safer to, when in doubt, not make a change. less chance of introducing a bug. we can always change it later if someone decides it isn't appropriate. i don't have particular feelings about the name as-is.

@kate-goldenring
Copy link
Contributor Author

Closing this. Consider reopening if request for name change occurs

@kate-goldenring kate-goldenring deleted the rename-webhook-configuration branch September 8, 2021 21:35
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