DemesneGH commented on issue #153: URL: https://github.com/apache/incubator-teaclave-trustzone-sdk/issues/153#issuecomment-2522101102
Thanks for the new proposal and I agree with the design. I have a few minor recommendations: 1. We can simplify the scenario by removing this: ``` // for people don't care about the response pub fn new_command<'a>(&'a self, command_id: u32) -> Command<'a> { Command::new(self, command_id) } ``` just keep the legacy `invoke()` and introduce the new `invoke_with_capacity()` with similar parameters (cmd, sub_cmd), I would suggest something like: ``` pub fn invoke_with_capacity(&self, command_id: u32, subcommand_id: u32, capacity: usize) ``` for developers: ``` const RESPONSE_LEN: usize = 100; let result_vec = plugin.invoke_with_capacity(command_id, subcommand_id, RESPONSE_LEN) .write_body(request) .call()?; ``` 2. For the PR: In terms of naming e.g `Command ` use a more specific name such as `LoadablePluginCommand` for distinguish. If there's nothing else to discuss regarding the issue, please feel free to open the PR. Thanks again! -- 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: dev-unsubscr...@teaclave.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@teaclave.apache.org For additional commands, e-mail: dev-h...@teaclave.apache.org