Skip to content

Fix fabric8 client dependency (related to issue 900) #905

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

Merged
merged 5 commits into from
Nov 9, 2021

Conversation

wind57
Copy link
Contributor

@wind57 wind57 commented Nov 5, 2021

No description provided.

@wind57
Copy link
Contributor Author

wind57 commented Nov 5, 2021

This PR should go after this one and then changes will make a lot more sense. I did not want to muddy the waters in that PR, that fixes an actual problem... When that one is merged, I'll comment on this one more if needed.

But basically the idea is the same as there, we track kubernetes-client dependency in our spring-cloud-kubernetes-dependencies, while it is provided by bom. And we do that for <scope>test</scope> and <type>test-jar</type>, which makes it even more compelling to remove it from there.

@wind57 wind57 changed the title Fix fabric8 client dependency Fix fabric8 client dependency (related to issue 900) Nov 5, 2021
@wind57 wind57 marked this pull request as ready for review November 5, 2021 10:58
</exclusions>
</dependency>

<dependency>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the actual change in this PR, we should drop kubernetes-client explicit dependency, it is managed by bom anyway

@ryanjbaxter
Copy link
Contributor

Seems like some of these changes duplicate whats in #904

@wind57
Copy link
Contributor Author

wind57 commented Nov 5, 2021

right, I branched from that one - because otherwise the build would have failed. That is why I said that if we agree on that one and merge it, and then look at this one, it will make more sense (the overlapping changes would not be present)

@@ -245,24 +245,6 @@
<scope>test</scope>
</dependency>

<dependency>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that now it makes a lot more sense - just like the mock server removal, we remove client also. It has the same <scope>test</scope>

@@ -84,6 +84,13 @@
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>
<!-- in favor of mockito-inline -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

some tests were failing, so need mockito-inline.

@wind57
Copy link
Contributor Author

wind57 commented Nov 9, 2021

@ryanjbaxter it should make a lot more sense now

@ryanjbaxter ryanjbaxter added this to the 2.1.0 milestone Nov 9, 2021
@ryanjbaxter ryanjbaxter merged commit 94f9fdc into spring-cloud:main Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants