spring-guides / gs-service-registration-and-discovery

Service Registration and Discovery :: Learn how to register and find services with Eureka
https://spring.io/guides/gs/service-registration-and-discovery/
Apache License 2.0
151 stars 192 forks source link

Rewrite all material including README and examples #33

Closed robertmcnees closed 4 months ago

robertmcnees commented 5 months ago

This PR is a near complete rewrite of the existing guide.

The existing guide was dated in several ways, most notably in a reference to @EnableDiscoveryClient (identified in issue #25) and boostrap.properties (identified in issue #28). Both of these methods are not available or recommended with the latest version of Spring Cloud. Rather than simply updating the existing applications to work, I took the opportunity to rethink the examples used.

A couple of key differences in this PR to what I replaced:

robertmcnees commented 5 months ago

Given the significant changes, I'm hoping to have a review from @OlgaMaciaszek.

codsaf commented 5 months ago

It is so a nice work!

codsaf commented 5 months ago

Can you tell me how the servicea and serviceb know the eureka server's ip address and port?

robertmcnees commented 4 months ago

@OlgaMaciaszek I made all of the changes you requested, with the exception of the defensive coding check I'd like to get your thoughts on.

For the Gradle/Maven discussion, that was a miss on my part. I usually try to include build files in the code for both Gradle and Maven. I like to only have a single set of commands reference in the README, and I should default to the preference of the underlying project. In this case, I should be using Maven since that is what Spring Cloud Netflix uses.

I added the Maven wrapper and pom.xml to all 3 projects. I also changed the README so that we use Maven ./mvnw spring-boot:run instead of Gradle ./gradlew bootRun.

OlgaMaciaszek commented 4 months ago

@robertmcnees there still seem to be some gradle files present in the project. Is this on purpose?

robertmcnees commented 4 months ago

@OlgaMaciaszek Yes, the inclusion of both gradle and maven is on purpose. I added both build systems here for consistency to our other guides which also include both Maven and Gradle build systems. The README should feature Maven commands, according to your preference.

I believe all of our guides originated with advice from this wiki about cloning this project. Several years ago the decision was made to have both build systems available in the guides.

If you feel strongly we should remove Gradle completely from this guide I can go that route. Personally, I'd prefer consistency if it doesn't add complexity for the learner. But I did break tradition here by removing the complete and initial folders because I felt that repeating all three applications in two places would be too confusing. If there is a similar argument to removing Gradle, then no problem for me to remove it.

From a sustainability perspective, I have GitHub Actions and Dependabot configured so I can manage both build systems simultaneously.

What do you think?

OlgaMaciaszek commented 4 months ago

Yes, it's good. May be helpful for the users to have both build systems.