[PR #762] [MERGED] Refine external reader API #719

New Issue

2025-12-30T01:26:22+01:00

adam commented

2025-12-30 01:26:22 +01:00

📋 Pull Request Information

Original PR: https://github.com/apple/pkl/pull/762
Author: @HT154
Created: 10/31/2024
Status: ✅ Merged
Merged: 11/1/2024
Merged by: @bioball

Base: main ← Head: external-reader-api-refinement

📝 Commits (1)

b117011 Refine external reader API

📊 Changes

16 files changed (+89 additions, -31 deletions)

View changed files

📝 pkl-cli/src/main/kotlin/org/pkl/cli/CliEvaluator.kt (+1 -1)
📝 pkl-cli/src/main/kotlin/org/pkl/cli/CliImportAnalyzer.kt (+1 -1)
📝 pkl-cli/src/main/kotlin/org/pkl/cli/CliTestRunner.kt (+1 -1)
📝 pkl-codegen-java/src/main/kotlin/org/pkl/codegen/java/CliJavaCodeGenerator.kt (+1 -1)
📝 pkl-codegen-kotlin/src/main/kotlin/org/pkl/codegen/kotlin/CliKotlinCodeGenerator.kt (+1 -1)
📝 pkl-core/src/main/java/org/pkl/core/Readers.java (+1 -1)
📝 pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcess.java (+14 -3)
📝 pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcessImpl.java (+21 -4)
📝 pkl-core/src/main/java/org/pkl/core/module/ExternalModuleResolver.java (+3 -2)
📝 pkl-core/src/main/java/org/pkl/core/module/ModuleKeyFactories.java (+10 -3)
📝 pkl-core/src/main/java/org/pkl/core/resource/ExternalResourceResolver.java (+5 -2)
📝 pkl-core/src/main/java/org/pkl/core/resource/ResourceReaders.java (+18 -8)
📝 pkl-core/src/main/java/org/pkl/core/service/ExecutorSpiImpl.java (+1 -1)
📝 pkl-core/src/main/resources/org/pkl/core/errorMessages.properties (+3 -0)
📝 pkl-core/src/test/kotlin/org/pkl/core/externalreader/TestExternalReaderProcess.kt (+7 -1)
📝 pkl-doc/src/main/kotlin/org/pkl/doc/CliDocGenerator.kt (+1 -1)

📄 Description

Thanks to @translatenix for identifying these in #759. This is the subset of items that were either low-hanging fruit or definitely need be addressed before this becomes public API.

Encapsulate message transport by removing ExternalReaderProcess.getTransport and adding getModuleResolver and getResourceResolver methods
Reuse Random instances within ExternalReaderProcessImpl and module/resource resolvers
Externalize all ExternalReaderProcessException messages
Add some missing doc comments to ModuleKeyFactories and ResourceReaders methods for external readers
Move org.pkl.core.util.Readers to org.pkl.core.Readers

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/apple/pkl/pull/762 **Author:** [@HT154](https://github.com/HT154) **Created:** 10/31/2024 **Status:** ✅ Merged **Merged:** 11/1/2024 **Merged by:** [@bioball](https://github.com/bioball) **Base:** `main` ← **Head:** `external-reader-api-refinement` --- ### 📝 Commits (1) - [`b117011`](https://github.com/apple/pkl/commit/b1170116276a8fa81bec56f9cd4ab8b3151884f9) Refine external reader API ### 📊 Changes **16 files changed** (+89 additions, -31 deletions) <details> <summary>View changed files</summary> 📝 `pkl-cli/src/main/kotlin/org/pkl/cli/CliEvaluator.kt` (+1 -1) 📝 `pkl-cli/src/main/kotlin/org/pkl/cli/CliImportAnalyzer.kt` (+1 -1) 📝 `pkl-cli/src/main/kotlin/org/pkl/cli/CliTestRunner.kt` (+1 -1) 📝 `pkl-codegen-java/src/main/kotlin/org/pkl/codegen/java/CliJavaCodeGenerator.kt` (+1 -1) 📝 `pkl-codegen-kotlin/src/main/kotlin/org/pkl/codegen/kotlin/CliKotlinCodeGenerator.kt` (+1 -1) 📝 `pkl-core/src/main/java/org/pkl/core/Readers.java` (+1 -1) 📝 `pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcess.java` (+14 -3) 📝 `pkl-core/src/main/java/org/pkl/core/externalreader/ExternalReaderProcessImpl.java` (+21 -4) 📝 `pkl-core/src/main/java/org/pkl/core/module/ExternalModuleResolver.java` (+3 -2) 📝 `pkl-core/src/main/java/org/pkl/core/module/ModuleKeyFactories.java` (+10 -3) 📝 `pkl-core/src/main/java/org/pkl/core/resource/ExternalResourceResolver.java` (+5 -2) 📝 `pkl-core/src/main/java/org/pkl/core/resource/ResourceReaders.java` (+18 -8) 📝 `pkl-core/src/main/java/org/pkl/core/service/ExecutorSpiImpl.java` (+1 -1) 📝 `pkl-core/src/main/resources/org/pkl/core/errorMessages.properties` (+3 -0) 📝 `pkl-core/src/test/kotlin/org/pkl/core/externalreader/TestExternalReaderProcess.kt` (+7 -1) 📝 `pkl-doc/src/main/kotlin/org/pkl/doc/CliDocGenerator.kt` (+1 -1) </details> ### 📄 Description Thanks to @translatenix for identifying these in #759. This is the subset of items that were either low-hanging fruit or definitely need be addressed before this becomes public API. * Encapsulate message transport by removing `ExternalReaderProcess.getTransport` and adding `getModuleResolver` and `getResourceResolver` methods * Reuse `Random` instances within `ExternalReaderProcessImpl` and module/resource resolvers * Externalize all `ExternalReaderProcessException` messages * Add some missing doc comments to `ModuleKeyFactories` and `ResourceReaders` methods for external readers * Move `org.pkl.core.util.Readers` to `org.pkl.core.Readers` --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

adam added the pull-request label 2025-12-30 01:26:22 +01:00

adam closed this issue

2025-12-30 01:26:22 +01:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/pkl#719