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

Reply via email to