JoaoJandre commented on PR #9208:
URL: https://github.com/apache/cloudstack/pull/9208#issuecomment-2252836604

   Hey @abhisar
   
   Good to see you actually have a specification for such a big feature. Sadly 
we have too many PRs that change 2k+ lines with little to none specification. 
   
   I have a few suggestions regarding the feature:
   
   1. On the `file_share` table you have on the spec, I think that we are 
missing a `created` column, to track when the file_share was created. I would 
also add a `removed` column, to track when it was removed.
   2. On your FSM, add a final state called `Expunged`, that is reached when 
the `ExpungeOperation` is successful.
   3. I know that using shell scripts or python might be tempting, but these 
tend to be hard to maintain and expand without breaking. I would advise on 
going back to the idea of doing something similar to what is done for the 
CPVM/SSVM. 


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to