Refactor builder pattern in VertexAiGeminiSafetySetting #3326
Conversation
Signed-off-by: jay <[email protected]>
mjkhub
force-pushed
the
refactor-builder-pattern-VertexAiGeminiSafetySetting
branch
from
4533c0c to
0e1a536
Compare
|
Boss, would really appreciate it if you could take a quick look at this PR 🫡 |
Signed-off-by: jay <[email protected]>
mjkhub
force-pushed
the
refactor-builder-pattern-VertexAiGeminiSafetySetting
branch
from
84e6164 to
3f96019
Compare
|
Boss, I’ve fixed the formatting issues using spring-javaformat:apply. |
|
@mjkhub Thanks for the PR using the builder! |
| this.category = HarmCategory.HARM_CATEGORY_UNSPECIFIED; | ||
| this.threshold = HarmBlockThreshold.HARM_BLOCK_THRESHOLD_UNSPECIFIED; | ||
| this.method = HarmBlockMethod.HARM_BLOCK_METHOD_UNSPECIFIED; | ||
| private VertexAiGeminiSafetySetting(Builder builder) { |
There was a problem hiding this comment.
Since we are changing the public accessor of this constructor to private, we need to deprecate this constructor by keeping this change. The deprecated method can further be removed right after 1.1.0 release.
There was a problem hiding this comment.
The ctor should not accept a builder, the builder should be calling the private ctor that contains the full argument list
There was a problem hiding this comment.
I've updated the code to use the full-argument constructor as you mentioned 🫡
As for deprecating the public constructor, I tried to add the annotation, but since the new one has the exact same parameters, it's not actually possible to keep both.
Signed-off-by: jay <[email protected]>
|
Thanks for the fixes @mjkhub will be merging soon. |
3 participants
Hey boss, I noticed a small part that could be cleaned up to better align with the builder pattern.
It’s not a big change, but I thought it might be helpful—please take a look when you have a chance.
Thanks 👍