stevedlawrence commented on code in PR #1237:
URL: https://github.com/apache/daffodil/pull/1237#discussion_r1600187172
##########
build.sbt:
##########
@@ -413,14 +413,28 @@ lazy val ratSettings = Seq(
)
lazy val unidocSettings = Seq(
- ScalaUnidoc / unidoc / unidocProjectFilter := inProjects(sapi, udf),
+ ScalaUnidoc / unidoc / unidocProjectFilter := inProjects(sapi, udf,
runtime1),
ScalaUnidoc / unidoc / scalacOptions := Seq(
"-doc-title",
"Apache Daffodil " + version.value + " Scala API",
"-doc-root-content",
(sapi / baseDirectory).value + "/root-doc.txt"
),
- JavaUnidoc / unidoc / unidocProjectFilter := inProjects(japi, udf),
+ ScalaUnidoc / unidoc / unidocAllSources :=
+ (ScalaUnidoc / unidoc / unidocAllSources).value.map { sources =>
+ sources.filter { source =>
+ //
+ // This filter limits the scaladoc to only files which have
+ // api, sapi, or japi as directory names in their paths.
+ //
+ val str = source.toString
+ str.contains("/udf/") ||
+ str.contains("/api/") ||
+ str.contains("/sapi/") ||
+ str.contains("/japi/")
Review Comment:
/japi/ doesn't exist in scaladoc. It doesn't hurt to have it I guess, and it
makes the logic the same, so maybe this is fine?
##########
build.sbt:
##########
@@ -431,9 +445,21 @@ lazy val unidocSettings = Seq(
),
JavaUnidoc / unidoc / unidocAllSources := (JavaUnidoc / unidoc /
unidocAllSources).value.map {
sources =>
- sources.filterNot { source =>
- source.toString.contains("$") ||
source.toString.contains("packageprivate")
- }
+ sources
+ .filterNot { source =>
+ source.toString.contains("$") ||
source.toString.contains("packageprivate")
+ }
+ .filter { source =>
+ //
+ // This filter limits javadoc to only files with
+ // api, sapi, or japi directories in their paths.
+ //
+ val str = source.toString
+ str.contains("/udf/") ||
+ str.contains("/api/") ||
+ str.contains("/sapi/") ||
+ str.contains("/japi/")
Review Comment:
Same thoughts as above, note that sapi doesn't exist in javadoc but probably
doesn't hurt to keep for consistency.
##########
build.sbt:
##########
@@ -431,9 +445,21 @@ lazy val unidocSettings = Seq(
),
JavaUnidoc / unidoc / unidocAllSources := (JavaUnidoc / unidoc /
unidocAllSources).value.map {
sources =>
- sources.filterNot { source =>
- source.toString.contains("$") ||
source.toString.contains("packageprivate")
- }
+ sources
+ .filterNot { source =>
+ source.toString.contains("$") ||
source.toString.contains("packageprivate")
+ }
+ .filter { source =>
+ //
+ // This filter limits javadoc to only files with
+ // api, sapi, or japi directories in their paths.
+ //
+ val str = source.toString
Review Comment:
The generated javadoc doesn't contain the metadata an other classes
mentioned above. I think because they are written in scala so would need the
genjavadoc plugin. Or do we want to exclude those and consider them
experimental? Maybe we should only allow "/layers/api/" or something?
##########
build.sbt:
##########
@@ -413,14 +413,27 @@ lazy val ratSettings = Seq(
)
lazy val unidocSettings = Seq(
- ScalaUnidoc / unidoc / unidocProjectFilter := inProjects(sapi, udf),
+ ScalaUnidoc / unidoc / unidocProjectFilter := inProjects(sapi, udf,
runtime1),
ScalaUnidoc / unidoc / scalacOptions := Seq(
"-doc-title",
"Apache Daffodil " + version.value + " Scala API",
"-doc-root-content",
(sapi / baseDirectory).value + "/root-doc.txt"
),
- JavaUnidoc / unidoc / unidocProjectFilter := inProjects(japi, udf),
+ ScalaUnidoc / unidoc / unidocAllSources :=
+ (ScalaUnidoc / unidoc / unidocAllSources).value.map { sources =>
+ sources.filter { source =>
+ //
+ // This filter limits the scaladoc to only files which have
+ // api, sapi, or japi as directory names in their paths.
+ //
+ val str = source.toString
+ str.contains("/api/") ||
Review Comment:
For reference, these are the new classes in the API:
```
root
org
apache
daffodil
runtime1
api
ChoiceMetadata
ComplexElementMetadata
DFDL
DFDLPrimType
ElementMetadata
InfosetArray
InfosetComplexElement
InfosetDocument
InfosetElement
InfosetItem
InfosetSimpleElement
InfosetTypeException
Metadata
MetadataHandler
ModelGroupMetadata
SequenceMetadata
SimpleElementMetadata
TermMetadata
layers
api
ChecksumLayer
Layer
```
I think all the Metadata ones are new public APIs (are they considered
experimental or are we officially supporting them an requiring backwards
compatibility?).
But I consider `DFDL` to be an internal API that people shouldn't use, at
least until we get rid of SAPI/JAPI with DAFFODIL-1747. Im not sure about the
others. Are any of these considered internal and should be filtered out?
##########
build.sbt:
##########
@@ -413,14 +413,27 @@ lazy val ratSettings = Seq(
)
lazy val unidocSettings = Seq(
- ScalaUnidoc / unidoc / unidocProjectFilter := inProjects(sapi, udf),
+ ScalaUnidoc / unidoc / unidocProjectFilter := inProjects(sapi, udf,
runtime1),
ScalaUnidoc / unidoc / scalacOptions := Seq(
"-doc-title",
"Apache Daffodil " + version.value + " Scala API",
"-doc-root-content",
(sapi / baseDirectory).value + "/root-doc.txt"
),
- JavaUnidoc / unidoc / unidocProjectFilter := inProjects(japi, udf),
+ ScalaUnidoc / unidoc / unidocAllSources :=
+ (ScalaUnidoc / unidoc / unidocAllSources).value.map { sources =>
+ sources.filter { source =>
+ //
+ // This filter limits the scaladoc to only files which have
+ // api, sapi, or japi as directory names in their paths.
+ //
+ val str = source.toString
Review Comment:
`str` contains an absolute path to the sources, so there's a rare edge case
that a directory that the daffodil repo is in contains one of these strings,
which would result in not filtering anything out. For example, a path like
`/home/user/api/repos/daffodil.git/`. I can't think of a reason why that would
happen, but who knows.
We could avoid this by doing something like:
```scala
val str = source.toString.stripPrefix(baseDirectory.value.toString)
```
--
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]
