gortiz commented on code in PR #10759:
URL: https://github.com/apache/pinot/pull/10759#discussion_r1196344690
##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/assignment/SegmentAssignmentConfig.java:
##########
@@ -34,6 +34,7 @@ public SegmentAssignmentConfig(@JsonProperty(value =
"segmentAssignmentStrategy"
_assignmentStrategy = assignmentStrategy;
}
+ @JsonProperty("segmentAssignmentStrategy")
Review Comment:
You can place `@JsonProperty` in the attribute. Alternatively, you can move
the `@JsonPropertyDescription` from the attribute to the getter.
`@JsonProperty` can be placed in attributes, getters and setters (well, and
attributes in `@JsonCreator`, but that the meaning there is different). In case
an annotation is found in several places, the preference will be getter >
setter > attribute. It doesn't mean that we should always place them in the
getter, but it means that we should not place annotations in different places
in order to make it more difficult to make mistakes.
My personal taste is to place them in attributes (as they are in the front
of the class) but I'm not going to block a PR that adds them in the getter
(especially if that is what we do in the rest of the code). But I really think
that having annotations in attributes and in getters is error prone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
