alamb commented on code in PR #25495:
URL: https://github.com/apache/datafusion/pull/25495#discussion_r4061276918
##########
docs/source/contributor-guide/api-health.md:
##########
@@ -43,6 +43,36 @@ Examples of non-breaking changes include:
- Marking a function as deprecated (`#[deprecated]`)
- Adding a new function to a `trait` with a default implementation
+### DataFusion-internal Public APIs
+
+Some APIs require `pub` visibility for use across DataFusion crates. Examples
+include:
+
+1. Test helpers.
+2. Operator APIs required by the optimizer to inspect or rewrite execution
plans
+ across crate boundaries.
+
+For APIs intended only for internal use, add `#[doc(hidden)]` and a doc comment
+section headed `# Public Only for Internal Use:`. Name the crate or component
+that requires access and explain why the API is not intended for downstream
use.
+For example:
+
+```txt
+impl HashTableLookupExpr {
+ /// ...
+ ///
+ /// # Public Only for Internal Use:
+ /// `datafusion-proto` tests require this constructor, but it is not part
of
+ /// the supported public API.
+ #[doc(hidden)]
+ pub fn new(...) {...}
+}
+```
+
+These APIs are hidden from generated documentation and are not considered part
Review Comment:
would recommend putting this detail in the intro
##########
docs/source/contributor-guide/api-health.md:
##########
@@ -43,6 +43,36 @@ Examples of non-breaking changes include:
- Marking a function as deprecated (`#[deprecated]`)
- Adding a new function to a `trait` with a default implementation
+### DataFusion-internal Public APIs
+
+Some APIs require `pub` visibility for use across DataFusion crates. Examples
Review Comment:
I recommend we also document "why" we have such APIs when so much of
DataFusion's flexibility is predicated on extensible APIs.
Maybe the idea is that these APIs are really meant as internal
implementation details, but for some logistical reason they have to be in
different crates. Thus the "internal" APIs aren't intended as extension points,
and may change / etc without warning?
##########
docs/source/contributor-guide/api-health.md:
##########
@@ -43,6 +43,36 @@ Examples of non-breaking changes include:
- Marking a function as deprecated (`#[deprecated]`)
- Adding a new function to a `trait` with a default implementation
+### DataFusion-internal Public APIs
+
+Some APIs require `pub` visibility for use across DataFusion crates. Examples
+include:
+
+1. Test helpers.
Review Comment:
I think in general multiple types of APIs just makes DataFUsion more
complicated and I would like to suggest we minimize the number of such APIs as
much as possible
For example, for test helpers, we could potentially use a feature
(`test_helpers` for example) to mark that they are not part of the public API
rather than marking them as a doc
--
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]
