Currently, we instantiate a builder by calling a constructor or a static method in a builder class:
Server server = new ServerBuilder()
.foo()
.bar()
.build();
I wonder if it was a good idea or we need to reconsider this decision in favor of having builder() method:
Server server = Server.builder()
.foo()
.bar()
.build();
The pros of builder() are:
'.', which is better in terms of aesthetics.ServerBuilder if we chain all.I like the Server.builder() way
Let me make the proposed change.
builder() or newBuilder()? Protobuf uses newBuilder() and Guava uses builder(). Which one is better?
+1.001 for builder vs +1 for newBuilder :)
Just came back from Spain and it reads like 1001 vs 1 :rofl:
I'm working on this
The remaining builder classes that need migration:
ClientBuilderDefaultClientBuilderParams ServerListenerBuilderHttpAuthServiceBuilderSimpleCompositeServiceBuilderSimpleCompositeRpcServiceBuilderCorsPolicyBuilderCorsServiceBuilderDocServiceBuilderHttpFileBuilderHttpFileServiceBuilderLoggingServiceBuilderJettyServiceBuilderArmeriaRetrofitBuilderKeyStoreCredentialResolverBuilderSamlServiceProviderBuilderTomcatServiceBuilderZooKeeperUpdatingListenerBuilderIt has been on progress since this monday. Apologize for for slow progress (__)
@4whomtbts It's good to be back! I think you need to know some changes in #2323.
1) Please make sure your current branch is up-to-date from the master branch. Lots of class which have HTTP keyword have been deprecated.
2) HttpAuthServiceBuilder and HttpFileServiceBuilder have been done, you may want to skip them.
3) This issue is scheduled to apply before release 1.0.0. Maybe in the middle of January. 馃槈
I'm looking forward to your next PR. Cheer up!
ClientBuilderDefaultClientBuilderParams ServerListenerBuilderHttpAuthServiceBuilder~SimpleCompositeServiceBuilderSimpleCompositeRpcServiceBuilderCorsPolicyBuilderCorsServiceBuilderDocServiceBuilderHttpFileBuilderHttpFileServiceBuilder~LoggingServiceBuilderJettyServiceBuilderArmeriaRetrofitBuilderKeyStoreCredentialResolverBuilderSamlServiceProviderBuilderTomcatServiceBuilderZooKeeperUpdatingListenerBuilder@4whomtbts It's good to be back! I think you need to know some changes in #2323.
- Please make sure your current branch is up-to-date from the master branch. Lots of class which have
HTTPkeyword have been deprecated.HttpAuthServiceBuilderandHttpFileServiceBuilderhave been done, you may want to skip them.- This issue is scheduled to apply before release 1.0.0. Maybe in the middle of January. 馃槈
I'm looking forward to your next PR. Cheer up!
ClientBuilderDefaultClientBuilderParamsServerListenerBuilder- ~
HttpAuthServiceBuilder~SimpleCompositeServiceBuilderSimpleCompositeRpcServiceBuilderCorsPolicyBuilderCorsServiceBuilderDocServiceBuilderHttpFileBuilder- ~
HttpFileServiceBuilder~LoggingServiceBuilderJettyServiceBuilderArmeriaRetrofitBuilderKeyStoreCredentialResolverBuilderSamlServiceProviderBuilderTomcatServiceBuilderZooKeeperUpdatingListenerBuilder
Yes. My local branch is currently up-to-date.
Thank you for fill me in!
Once #2352 is merged, these are probably the last:
They are somewhat trickier than others, so let me handle them by myself.
Need to replace constants with factory methods as well:
EventCount.ZEROClientCacheControl.*ServerCacheControl.*PrometheusMeterRegistries.defaultRegistryAuthTokenExtractors.*CorsConfig.DISABLED.. and make non-final public classes final if not meant for extension.
Once #2352 is merged, these are probably the last:
- ClientBuilder
- DefaultClientBuilderParams
- new SimpleCompositeService(...)
- new SimpleCompositeRpcService(...)
- new DocService()
- new ArmeriaRetrofitBuilder(...)
- KeyStoreCredentialResolverBuilder
They are somewhat trickier than others, so let me handle them by myself.
Ok. I got it.
Most helpful comment
@4whomtbts It's good to be back! I think you need to know some changes in #2323.
1) Please make sure your current branch is up-to-date from the master branch. Lots of class which have
HTTPkeyword have been deprecated.2)
HttpAuthServiceBuilderandHttpFileServiceBuilderhave been done, you may want to skip them.3) This issue is scheduled to apply before release 1.0.0. Maybe in the middle of January. 馃槈
I'm looking forward to your next PR. Cheer up!
ClientBuilderDefaultClientBuilderParamsServerListenerBuilderHttpAuthServiceBuilder~SimpleCompositeServiceBuilderSimpleCompositeRpcServiceBuilderCorsPolicyBuilderCorsServiceBuilderDocServiceBuilderHttpFileBuilderHttpFileServiceBuilder~LoggingServiceBuilderJettyServiceBuilderArmeriaRetrofitBuilderKeyStoreCredentialResolverBuilderSamlServiceProviderBuilderTomcatServiceBuilderZooKeeperUpdatingListenerBuilder