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

Reply via email to