Contributor
Advances #1029 adding thread safety on the mutable variables of the class (conversations and timeout_jobs).
The original issue also asked to internalize all the other properties but it would incur breaking changes and a lot of refactoring, I'll leave this task to someone more proficient with the codebase.
Member
Hey man, this looks good to me, but there should be definitly someone else to look over this cough cough @Eldinnie? cough cough. Can you remove the #Todo line though and either put it in this conversation or in the related issue? Then codacy should be fine with it.
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you address the TODO, I think this is good
Contributor Author
I've addressed the todo, should I squash the commits?
Sorry for the trouble, I wanted to help you a bit but It's my first time doing a PR in a big repo 😅.
Member
No need to, we will before merging into master. Just go ahead and make as many commits as you want :)
Member
Contributor Author
I've been studying the test code and I noticed something strange: the assignment at line 494 (that has moved to 552 in the master branch) is duplicated, it seems like a merge error or a typo.
Member
@SnowyCoder Interesting find ^^
Edit: e75615c introduced it, not quite sure what was going on here...
Member
Contributor Author
Thank you for the support!