DaanHoogland commented on PR #8484:
URL: https://github.com/apache/cloudstack/pull/8484#issuecomment-1886636007

   > > @anniejili , it makes function sense what you are trying to do. I have 
some requests though.
   > > 
   > > * can you make methods for the new code, especially try to avoid having 
nested try-catch blocks in a single method?
   > 
   > @DaanHoogland Hi Dahn, Thank you for reviewing my code. As far as I can 
tell, there is no nested try-catch introduced by this change. Can you please 
double check?
   
   You are right,I cannot see that nesting of try-catch blocks anymore, maybe I 
was confusing different PRs. The request remains however; can you make methods 
for the new code? for readability sake, verbs and nouns are easier to read 
(given proper method naming) than curls and braces. Also this commitUserVm 
method is now 150 lines long (not your fault) so let's reduce that a bit.
   
   In no way is this a deprecation of your code, just a suggestion for 
improvement. (not a :-1: !!)


-- 
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