remove post files after a restart #30
Conversation
|
The pull request #28 was an error, nothing was applied. Here the correct commit for this feature. |
| @@ -645,3 +653,48 @@ def __remove_list_files(self): | |||
| os.remove(f) | |||
| except WindowsError: | |||
| pass | |||
|
|
|||
| # | |||
| def __remove_post_results_files(self, step_label): | |||
There was a problem hiding this comment.
Why you are using the double underline "__" for this method? The naming convention of Python here says that it would do the name mangling to the class name which I'm not sure is what you intended.
There was a problem hiding this comment.
Yes you are right, it must be only a single underline for an internal method. I was following the notation or style found in the script. Maybe some of the internal methods have to be revised too. @jcotela
There was a problem hiding this comment.
Well, I deliberately used double leading underscore for internal stuff (things that, if I was in C, I would declare as private functions). If you prefer to avoid name mangling, this is something I would be open to discuss.
I agree that this kind of magical python stuff should not be used without a good reason, however.
There was a problem hiding this comment.
i think that __ is for builtings, like __ init __()
if you want to show something is private it is 1 underscore at the beginning.
still, my proposal would be to accept it for now...
Riccardo
There was a problem hiding this comment.
The double underline __ is not something new in this file. If it is not desired it would be better to correct it in all methods after this merge. Because this is mainly a file of @jcotela, maybe he can change it carefully later on.
There was a problem hiding this comment.
I agree with @josep-m-carbonell that we shouldn't stop this commit until we agree on a naming convention. I'm giving my OK and creating a new issue (#42) to continue the discussion.
There was a problem hiding this comment.
Thank you Jordi 👍
remove post files after a restart, so lists are rebuild correctly