Conversation
|
This PR was not deployed automatically as @hacke2 does not have access to the Railway project. In order to get automatic PR deploys, please add @hacke2 to the project inside the project settings page. |
Walkthrough此次变更主要集中在扩展插件 API 的集成与管理上，通过引入新的接口和模块，增强了外部进程和 API 的配置及管理能力。这些改动使得应用程序的可扩展性更强，便于与外部系统的集成，同时优化了日志记录功能，提升了整体架构的灵活性和可维护性。 Changes
Assessment against linked issues
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (2)
Additional comments not posted (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (4)
packages/extension/src/hosted/extension-log2.ts (1)
23-24: 注释说明注释解释了代码的设计选择，提供了有用的上下文信息。
考虑将注释翻译为英文，以便于非中文开发者理解。
- // 这块不是非常合理，插件进程引用了 Node 主进程的代码，先改为多例方式防止 AppConfig 找不到的问题 + // This is not very reasonable, the plugin process references the Node main process code, temporarily changed to multi-instance to prevent AppConfig from not being found.packages/logs-core/src/node/log-manager.ts (1)
29-29: 确保appConfig属性的安全访问在
LogServiceManager中，appConfig的属性logDir和logLevel被直接使用，没有进行空值检查或提供默认值。建议添加相应的检查或默认值，以避免潜在的运行时错误。
logDir: 需要检查是否为 null 或 undefined。logLevel: 需要检查是否为 null 或 undefined。请确保在使用这些属性时进行适当的验证，以提高代码的健壮性。
Analysis chain
确保
appConfig的安全使用
appConfig现在是可选的，这意味着在使用appConfig的属性之前需要进行检查，以避免运行时错误。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify safe usage of `appConfig` in `LogServiceManager`. # Test: Search for `appConfig` usage. Expect: Presence of null checks or default values. rg --type typescript -A 3 $'this\.appConfig\.' packages/logs-core/src/node/log-manager.tsLength of output: 125
Script:
#!/bin/bash # Description: Verify safe usage of `appConfig` in `LogServiceManager`. # Test: Search for `appConfig` usage. Expect: Presence of null checks or default values. rg --type ts -A 3 $'this\.appConfig\.' packages/logs-core/src/node/log-manager.tsLength of output: 383
packages/extension/src/hosted/api/sumi/ext.host.api.impl.ts (1)
62-71: 建议在使用createApiFactory之前进行检查在处理
externalSumiExtApi时，虽然已经对identifier进行了存在性检查，但createApiFactory未进行类似的检查。建议在调用createApiFactory之前，确保其存在以避免潜在的未定义错误。
- 代码位置:
packages/extension/src/hosted/api/sumi/ext.host.api.impl.ts, 行 62-71Analysis chain
验证
externalSumiExtApi的处理逻辑在处理
externalSumiExtApi时，确保identifier和createApiFactory的存在性，以避免潜在的未定义错误。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify handling of `externalSumiExtApi` keys for presence of `identifier` and `createApiFactory`. # Test: Search for `externalSumiExtApi` key usage. Expect: Checks for `identifier` and `createApiFactory`. rg --type typescript -A 3 $'externalSumiExtApi\[key\]\.' packages/extension/src/hosted/api/sumi/ext.host.api.impl.tsLength of output: 152
Script:
#!/bin/bash # Description: Verify handling of `externalSumiExtApi` keys for presence of `identifier` and `createApiFactory`. # Test: Search for `externalSumiExtApi` key usage and ensure checks for `identifier` and `createApiFactory`. rg -A 3 'externalSumiExtApi\[key\]\.' packages/extension/src/hosted/api/sumi/ext.host.api.impl.tsLength of output: 333
packages/extension/src/hosted/api/vscode/ext.host.command.ts (1)
15-15: 导入路径更改验证
IBuiltInCommand的导入路径更改为../../../common/ext.process是正确的，因为该接口在此位置定义。然而，tools/dev-tool/src/ext-host.ts文件中仍然使用旧路径@opensumi/ide-extension/lib/hosted/ext.process-base，这可能导致错误或不一致。请检查并更新此文件中的导入路径。
tools/dev-tool/src/ext-host.ts: 旧路径@opensumi/ide-extension/lib/hosted/ext.process-base需要更新。Analysis chain
验证导入路径更改的影响。
IBuiltInCommand的导入路径从../../ext.process-base更改为../../../common/ext.process。请确保新路径正确，并验证此更改是否影响到文件中的其他功能。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of the import path change for `IBuiltInCommand`. # Test: Search for all usages of `IBuiltInCommand` in the repository to ensure the new path is correct. rg --type ts 'IBuiltInCommand'Length of output: 677
|
实现逻辑我看起来OK 这个单测挂了 packages/extension/tests/hosted/api/vscode/ext.host.test.ts |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
packages/extension/src/common/sumi/index.ts (1)
39-50: 设计良好但需注意文档。
SumiApiExtender类设计用于扩展API，使用了抽象类和可选方法。建议为createRPCService和createApiFactory方法添加详细的文档注释，以便开发者更好地理解其用途和使用方式。/** * create rpc service when main thread could call */ createRPCService?: () => [identifier: ProxyIdentifier<T>, service: T]; /** * api factory * can use rpc service to call main thread */ createApiFactory: (service?: T) => any;
|
是否要在 startup 里面加入一些简答示例代码？ |
|
/next |
|
🎉 PR Next publish successful! 3.2.4-next-1723688727.0 |
hacke2
changed the title
|
LGTM |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3933 +/- ##
==========================================
- Coverage 54.89% 54.88% -0.01%
==========================================
Files 1566 1568 +2
Lines 95573 95632 +59
Branches 19602 19608 +6
==========================================
+ Hits 52461 52488 +27
- Misses 35794 35825 +31
- Partials 7318 7319 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
* refactor: replace AppConfig with ExtHostAppConfig * feat: support register external api to exthost * refactor: abstract external extension API * feat: support reguster main thread extender * fix: fix lint * fix: missing ExtHostAppConfig for test
3 participants
Types
Background or solution
resolve #3932
Changelog
Summary by CodeRabbit
Summary by CodeRabbit
新功能
优化