Skip to content

Conversation

@onobc
Copy link
Contributor

@onobc onobc commented Oct 21, 2024

This PR does the following:

  • Adds a spring-grpc-spring-boot-starter module as described here.
  • Adjusts the scope of the dependencies in the spring-grpc-autoconfigure module as recommended here.

At first glance, the autoconfigure is simple enough to just combine the starter + autoconfigure in a single module. However, I can envision us having multiple starters in the future (netty, shaded netty, okio, etc...).

As it currently stands, our default opinion is "netty" over "shaded netty".

  1. Should that be our default?
  2. Should we go ahead and name this default starter w/ according suffix, something like the following:?
  • spring-grpc-spring-boot-starter-shaded-netty
  • spring-grpc-spring-boot-starter-netty
  • spring-grpc-spring-boot-starter-okio

This commit does the following:

* Adds a `spring-grpc-spring-boot-starter` module

* Adjusts the scope of the dependencies in the`spring-grpc-spring-boot-autoconfigure`
  module to optional
@onobc onobc requested a review from dsyer October 21, 2024 03:02
@dsyer
Copy link
Member

dsyer commented Oct 21, 2024

This change was coming sooner or later, so I suppose it might as well be now.

Personally I like to have a default (no need to engage brain). I prefer netty to shaded netty just because of awkwardness with IDEs not showing source code - also we'd like to get it working in a webflux app so we'd want the same netty as reactor-netty for that. So I can see why explicitly naming all the starters (and no default) makes sense, but I just can't make myself like it.

The downside to having a default is that users have to explicitly exclude it if they want to change (like tomcat and jetty in Spring Boot).

@dsyer dsyer merged commit d77f7a0 into spring-projects:main Oct 21, 2024
2 checks passed
@onobc
Copy link
Contributor Author

onobc commented Oct 21, 2024

I am good w/ the no suffix as the default as well. This is what spring-cloud-gateway and others do as well. We can append suffixes as needed when the time comes. Thanks for the review and merge.

@onobc onobc deleted the add-starter-module branch November 7, 2024 16:11
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.

2 participants