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

fix #4460 removing the split packages #4464

Merged
merged 4 commits into from Oct 18, 2022
Merged

Conversation

shawkins
Copy link
Contributor

@shawkins shawkins commented Sep 29, 2022

Description

Fix #4460

To support java modules (#4460) and cleaner OSGI support split packages need to be removed. Since we've now had 6.0 / 6.1 releases with the relevant client classes either explicitly or effectively deprecated we should be able to resolve this for splits that were introduced by the client/api refactorings.

kubernetes-client:

  • io.fabric8.kubernetes.client -> io.fabric8.kubernetes.client.impl
  • io.fabric8.kubernetes.client.internal -> io.fabric8.kubernetes.client.dsl.internal (only 3 classes)

openshift-client:

  • io.fabric8.openshift.client -> io.fabric8.openshift.client.impl

Additionally a still deprecated DefaultKubernetesClient and DefaultOpenShiftClient adapters were added to the -api modules for continued compatibility when not using a builder. The actual implementations have been renamed XXXClientImpl.

I tested against a sample module project and this seemed to be sufficient to address the issue. There may be some more additional changes needed for OSGI - in particular the osgi.export and osgi.private should be reviewed for kubernetes-client and openshift-client @rohanKanojia is this something that you are familiar with?

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

Copy link
Member

@manusa manusa left a comment

Choose a reason for hiding this comment

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

LGTM

I'd prefer to see something like VanillaKubernetesClient instead of KubernetesClientImpl, but that's something very personal (I hate the Impl suffix in class names).

@@ -13,11 +13,14 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.fabric8.kubernetes.client;
package io.fabric8.kubernetes.client.impl;
Copy link
Member

Choose a reason for hiding this comment

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

The test class name should probably be renamed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@shawkins
Copy link
Contributor Author

shawkins commented Oct 4, 2022

I'd prefer to see something like VanillaKubernetesClient instead of KubernetesClientImpl, but that's something very personal (I hate the Impl suffix in class names).

Feel free to use whatever naming you want - I don't have a strong preference. We'll probably want to re-assess the package names as well at some point. We could just have io.fabric8.kubernetes.client.(impl or internal) as the root package, rather than the two impls and two internals that will be there after this pr.

@sonarcloud
Copy link

sonarcloud bot commented Oct 6, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

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

27.2% 27.2% Coverage
1.6% 1.6% Duplication

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.

java.lang.ClassNotFoundException: io.fabric8.kubernetes.client.DefaultKubernetesClient
2 participants