uros-db commented on code in PR #56205:
URL: https://github.com/apache/spark/pull/56205#discussion_r3334778356


##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/SparkDateTimeUtils.scala:
##########


Review Comment:
   PR description vs. code: "package-private APIs". The description says the 
new methods are package-private, but they're declared as plain public defs on 
the public trait SparkDateTimeUtils (extended by the public object 
DateTimeUtils). So they actually become public API surface.
   
   It's consistent with the existing siblings (stringToTimestamp, etc., which 
are also public), so this isn't wrong per se — but given the feature is 
@Unstable and not yet wired to anything, consider tightening visibility (e.g. 
`private[sql]`) to avoid committing to a public contract prematurely. At 
minimum, the description should be corrected so reviewers aren't misled.



-- 
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]

Reply via email to