ashvina commented on code in PR #653: URL: https://github.com/apache/incubator-xtable/pull/653#discussion_r1960617738
########## xtable-api/src/main/java/org/apache/xtable/model/storage/InternalDataFile.java: ########## @@ -33,17 +38,18 @@ * * @since 0.1 */ -@Builder(toBuilder = true) -@Value -public class InternalDataFile { - // physical path of the file (absolute with scheme) - @NonNull String physicalPath; +@SuperBuilder(toBuilder = true) +@FieldDefaults(makeFinal = true, level = lombok.AccessLevel.PRIVATE) +@ToString(callSuper = true) +@EqualsAndHashCode(callSuper = true) +@Accessors(fluent = true) Review Comment: I find the fluent accessors less verbose, which makes the code easier to read. In the projects I am involved in, it is a preferred pattern, so I used it here. However, after creating the PR, I realized it makes our API inconsistent. I'm supportive of holding off this change for now and discussing it. If the community likes it, we can apply the change in one PR for all APIs. WDYT? -- 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...@xtable.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org