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 webhooks errors exported funcs #925

Merged
merged 3 commits into from Feb 10, 2023

Conversation

mihaialexandrescu
Copy link
Contributor

@mihaialexandrescu mihaialexandrescu commented Feb 2, 2023

Q A
Bug fix? no
New feature? no
API breaks? yes
Deprecations? no
Related tickets
License Apache 2.0

What's in this PR?

This PR renames the exported error checking functions from pkg webhooks to names that include the IsAdmission... prefix, rather than Is..., to hopefully make it clearer that they relate to admission webhooks errors and results.

The change is initially done in 2 commits :

  1. the non-breaking renames -- those functions were added very recently in Augment error behaviour #920 (after the v0.22.0 release)
  2. a breaking change -- to keep the naming scheme consistent in the package, the IsInvalidReplicationFactor function (which already exists in release v0.22.0) is renamed following the same pattern

Why?

More clarity and consistency in the naming scheme in the "webhooks" package

API-breaking markers ?

I am not sure what markers I need to use to correctly flag the breaking change in API, other than the table at the top of the PR description.

Still one question about renaming ...

Should the IsErrorDuringValidation function follow the same pattern since it includes the word validation already ... ? (aka : IsAdmissionErrorDuringValidation)
Currently, I favour renaming throughout and it is done for this one too.

@mihaialexandrescu mihaialexandrescu force-pushed the rename_webhooks_errors_exported_funcs branch from fa64fbc to 1cb447c Compare February 2, 2023 17:23
@mihaialexandrescu mihaialexandrescu force-pushed the rename_webhooks_errors_exported_funcs branch from 1cb447c to c7d1fe6 Compare February 9, 2023 15:17
@mihaialexandrescu mihaialexandrescu marked this pull request as ready for review February 9, 2023 15:26
@mihaialexandrescu mihaialexandrescu requested a review from a team as a code owner February 9, 2023 15:26
@pregnor
Copy link
Member

pregnor commented Feb 9, 2023

Should the IsErrorDuringValidation function follow the same pattern since it includes the word validation already ... ? (aka : IsAdmissionErrorDuringValidation)

I vote yes

@mihaialexandrescu mihaialexandrescu merged commit 8c76627 into master Feb 10, 2023
@mihaialexandrescu mihaialexandrescu deleted the rename_webhooks_errors_exported_funcs branch February 10, 2023 13:29
bartam1 pushed a commit that referenced this pull request Feb 17, 2023
* pkg webhooks: update new error functions names to IsAdmission prefix; NOT API breaking

* pkg webhooks: update IsInvalidReplicationFactor exported func name with IsAdmission prefix; API breaking change
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.

None yet

3 participants