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

Moving readiness and adding changelog for initial api changes #3833

Merged
merged 3 commits into from
Feb 15, 2022

Conversation

shawkins
Copy link
Contributor

Description

Followup to the api split #3654 to move readiness to resolve #2838

The proposal here is for a breaking change, users would need to update the packages.

That will leave just client.internal as an internal package in the api module. It has several utility classes: https://github.com/fabric8io/kubernetes-client/tree/master/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/internal

@manusa @rohanKanojia do you want move those as well, or leave them internal? And what about #1285

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

@rohanKanojia
Copy link
Member

I think we should move KubernetesDeserializer out of internal package. I would leave rest of the classes there as it is since no one really requested exposing them.

separate documentation will be needed over the usage of the api modules,
but that can be done once we've finalized how the user is creating the
client instances
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

31.2% 31.2% Coverage
0.0% 0.0% Duplication

@manusa
Copy link
Member

manusa commented Feb 15, 2022

It has several utility classes:

I wouldn't expose any of those classes. They are used internally (, and IMHO need some refactoring)

And what about #1285

I think it might be better to expose a KubernetesClient#registerCustomKind, if this is something that we want.

Comment on lines +17 to +21
* Refactoring #3654:
* Removed deprecated KubernetesClient.customResource / RawCustomResourceOperationsImpl, please use the generic resource api instead
* Removed deprecatedHttpClientUtils.createHttpClient(final Config config, final Consumer<OkHttpClient.Builder> additionalConfig), please use the OkHttpClientFactory instead
* Removed deprecated methods on SharedInformerFactory dealing with the OperationContext
* Refactoring #2838: Readiness/OpenShiftReadiness moved from client.internal.readiness to client.readiness
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

ohh
next file 😅

Copy link
Member

Choose a reason for hiding this comment

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

Since this list is bound to grow off limits, maybe it's better to just link to the migration guide WDYT?

@manusa manusa added this to the 6.0.0 milestone Feb 15, 2022
@manusa manusa merged commit 71153e0 into fabric8io:master Feb 15, 2022
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.

Readiness utility should not be flagged as internal
3 participants