Add exception handling to os.remove in the remove-asset-by_pathfang method of the avocado/utilities/asset.py file #5979
Conversation
|
Dear contributor,
The feature freeze will be active until the release planned on 28 June 2024. |
faker-king
force-pushed
the
mtt
branch
3 times, most recently
from
24d7540
to
6c660eb
Compare
|
hi @richtja, Can you help me see if I can merge the code |
There was a problem hiding this comment.
Hi @faker-king please remove 9806712 and 1051bc0. IMO, they are not related to this PR. To be honest, I am not sure about usage of pathlib in one specific method of asset utility. Correct me if I am wrong, but I don't see any additional benefit in this change.
But you catch that we don't have exception handling for this file manipulation, and that would be a great update. Therefore, please add only the exception handling. Thanks
faker-king
force-pushed
the
mtt
branch
2 times, most recently
from
ebaa13f
to
ee32a50
Compare
|
Hi @richtja, I am very sorry for the delay in modifying this PR. I have already made the necessary changes as per your request. Could you please double check. |
There was a problem hiding this comment.
Hi @faker-king, thank you for your updates. The overall changes LGTM, but you still have 9806712 and 1051bc0 commit which are not related to this PR, please drop those.
Also, can you please update the title of ee32a50, from your description you are updating the exception handling to all os.remove calls, but this commit is only about avocado/utils/asset.py. Please be more specific. Thank you.
faker-king
changed the title
|
@richtja Due to some permission issues with my account, it was verified and I have made the necessary modifications as per your suggestion to see if it can be merged |
|
Hi @faker-king, thank you for your updates. I'm still missing the commit title update, your changes don't add exception handling to os.remove in the whole avocado project, it is located only to avocado/utils/asset.py. Can you please update the title to be more specific? Thank you. |
faker-king
changed the title
|
@richtja Modified |
Hi @faker-king, sorry but maybe I haven't expressed myself clearly, I was talking about commit headline. It is needed for easier search thought changes in commit history in the future. Can you please update the commit headline as well, thanks. |
…ethod of the avocado/utilities/asset.py file Signed-off-by: mataotao <[email protected]>
|
@richtja Haha, it's okay, I didn't understand either, it has been modified. |
Using pathlib can make file path operations more object-oriented and easy to understand, and have better compatibility across different operating systems.