You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "olabusayoT (via GitHub)" <gi...@apache.org> on 2025/07/29 16:43:10 UTC
[PR] Use Seqs internally for BlobPaths/Diagnostics [daffodil]
olabusayoT opened a new pull request, #1516:
URL: https://github.com/apache/daffodil/pull/1516
- the Java API uses Java Lists, but we need immutability and O(1) prepends in the scala code, so we update to use seqs internally and leverage .asJava/.asScala wrappers as necessary. For getBlobPaths/getDiagnostics, we keep those as lists internally as well since they override the java functions.
DAFFODIL-3014
--
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-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Seqs internally for BlobPaths/Diagnostics [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1516:
URL: https://github.com/apache/daffodil/pull/1516#discussion_r2240802646
##########
daffodil-core/src/main/scala/org/apache/daffodil/lib/oolag/OOLAG.scala:
##########
@@ -450,7 +450,7 @@ object OOLAG {
final def warnings: Seq[api.Diagnostic] = oolagRoot.warnings_.toSeq
override def getDiagnostics: java.util.List[api.Diagnostic] =
- diagnostics
+ diagnostics.asJava
Review Comment:
Yeah, that could certainly cause confusion. I guess also OOLAG is only used during compile time where speed is important but not as critical. So this is probably fine, not worth worrying about unless it shows up in profiling.
--
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-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Seqs internally for BlobPaths/Diagnostics [daffodil]
Posted by "olabusayoT (via GitHub)" <gi...@apache.org>.
olabusayoT merged PR #1516:
URL: https://github.com/apache/daffodil/pull/1516
--
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-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Seqs internally for BlobPaths/Diagnostics [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1516:
URL: https://github.com/apache/daffodil/pull/1516#discussion_r2240505286
##########
daffodil-core/src/main/scala/org/apache/daffodil/lib/oolag/OOLAG.scala:
##########
@@ -450,7 +450,7 @@ object OOLAG {
final def warnings: Seq[api.Diagnostic] = oolagRoot.warnings_.toSeq
override def getDiagnostics: java.util.List[api.Diagnostic] =
- diagnostics
+ diagnostics.asJava
Review Comment:
OOLAG is an internal thing, so I would think we want to keep its diagnostics as a Seq. Ideally, java.util.List would be used on the public API boundary and not internally.
I wonder if we need two kinds of WithDiagnostics? The internal iapi.WithDiagnostics would return a Seq, and the public api.WithDiagnostics would return a List? And we would have to be careful to ensure internal things implement iapi.WithDiagnostics and public things implement api.WithDiagnostics. I wonder if that would cause issues or run into any conflicts where something needs to implement both iapi and api WithDiagnostics and would cause issues.
Probably isn't a big deal if it causes cascading issues.
--
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-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Seqs internally for BlobPaths/Diagnostics [daffodil]
Posted by "olabusayoT (via GitHub)" <gi...@apache.org>.
olabusayoT commented on code in PR #1516:
URL: https://github.com/apache/daffodil/pull/1516#discussion_r2240715218
##########
daffodil-core/src/main/scala/org/apache/daffodil/lib/oolag/OOLAG.scala:
##########
@@ -450,7 +450,7 @@ object OOLAG {
final def warnings: Seq[api.Diagnostic] = oolagRoot.warnings_.toSeq
override def getDiagnostics: java.util.List[api.Diagnostic] =
- diagnostics
+ diagnostics.asJava
Review Comment:
Hmm, we do have cases where an object implements both, but I think my main opposition will be lack of consistency where sometimes getDiagnostics returns Seq if it's using the internal PF, or if it's using the api PF, it returns a List...are we ok with this?
--
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-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org