rdblue commented on code in PR #16689:
URL: https://github.com/apache/iceberg/pull/16689#discussion_r3391248454
##########
core/src/main/java/org/apache/iceberg/TrackingBuilder.java:
##########
@@ -99,35 +98,54 @@ private TrackingBuilder(Tracking source, long
newSnapshotId) {
this.replacedPositions = null;
}
- /** Indicates that the DV has been updated for the new Tracking. */
+ /**
+ * Records that the file's DV was updated by this commit, advancing {@code
dvSnapshotId} to the
+ * commit snapshot. An EXISTING entry transitions to MODIFIED; an ADDED
entry stays ADDED (a file
+ * added and given a DV in the same commit).
Review Comment:
I think the original comment was correct and better than this one.
It's the LLM's tendency to over-document, but that's not a good practice. It
just means more churn when code is modified, as you can see here.
This update also doesn't consider what the caller needs to know. The caller
needs to know when to call `dvUpdated` and what it means. The caller does not
need to know all of the things that we decide to do to the internal state when
it's called. That's leaking details outside of our abstraction.
Here's an instruction that I give to my agents:
> Javadoc comments should describe the function or purpose of a class or
method, not restate its implementation. Only describe what is needed to
understand how to use the component, as though it is an interface or method
defined by an interface. Do not leak implementation details.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
