NOJIRA: Proposal for New Package Manager APIs#723
Conversation
Documentation Auto-GeneratedDocumentation has been automatically generated for the changed plugins and committed to this PR branch. The generated documentation files have been added to the PR for review. |
…l/entservices-apis into feature/package-manager-update
Documentation Auto-GeneratedDocumentation has been automatically generated for the changed plugins and committed to this PR branch. The generated documentation files have been added to the PR for review. |
There was a problem hiding this comment.
Pull request overview
This PR introduces new API methods to the PackageManager interface for handling installed package configurations and app secrets. The changes reorganize the API documentation by moving some method definitions and adding new functionality.
Changes:
- Added
getConfigForInstalledPackagemethod to retrieve configuration for already installed packages - Added
GetAppSecretsmethod to retrieve application secrets - Reorganized API documentation by moving
lockandunlockmethod definitions to later in the document
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| docs/apis/PackageManager.md | Adds new getConfigForInstalledPackage API documentation, reorganizes method ordering by moving lock and unlock sections, and updates example ID sequences |
| apis/PackageManager/IAppPackageManager.h | Adds interface definitions for GetConfigForInstalledPackage and GetAppSecrets methods, removes JSON version comment |
docs/apis/PackageManager.md
Outdated
| | result.config.fkpsFiles | std::string | json array of strings | | ||
| | result.config.fireboltVersion | std::string | | | ||
| | result.config.enableDebugger | bool | | | ||
| | result.config | string | metadata of package | |
There was a problem hiding this comment.
The return type changed from a structured RuntimeConfig object to an opaque String. This is a breaking change that removes type safety and makes it harder for API consumers to work with the configuration data. Consider maintaining the structured object return type or providing a clear migration path.
| | result.config | string | metadata of package | | |
| | result.config | RuntimeConfig | metadata of package | |
| // @param packageId: Package Id | ||
| // @param version: Version | ||
| // @param config: metadata of package | ||
| virtual Core::hresult GetConfigForInstalledPackage(const string &packageId, const string &version, string& config /* @out @opaque */) = 0; |
There was a problem hiding this comment.
The brief and text annotations repeat 'getConfigForInstalledPackage' without explaining what this method does differently from GetConfigForPackage. Add a description explaining that this retrieves configuration for packages that are already installed on the system, and clarify why it returns an opaque string instead of a structured RuntimeConfig.
| virtual Core::hresult GetAppSecrets( | ||
| const string &packageId, | ||
| const string &version, | ||
| string &secrets /* @out @opaque */ | ||
| ) = 0; |
There was a problem hiding this comment.
The GetAppSecrets method lacks documentation about what constitutes 'App Secrets' and how they should be handled securely. Add documentation explaining the format of the secrets return value, security considerations for handling this sensitive data, and any encryption or protection mechanisms in place.
… feature/package-manager-update
…l/entservices-apis into feature/package-manager-update
Documentation Auto-GeneratedDocumentation has been automatically generated for the changed plugins and committed to this PR branch. The generated documentation files have been added to the PR for review. |
No description provided.