fix: reporters receive copy of message#7620
fix: reporters receive copy of message#7620Pierre-Sassoulas merged 6 commits intopylint-dev:mainfrom Smixi:main
Conversation
Pull Request Test Coverage Report for Build 3260364257
💛 - Coveralls |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| @@ -0,0 +1,3 @@ | |||
| Message send to reporter are now copied so a reporter cannot modify the message sent to other reporters. | |||
|
|
|||
| Refs #7214 | |||
There was a problem hiding this comment.
should this be Closes?
There was a problem hiding this comment.
I wasn't sure of semantics of this, but yes ! Let me fix that.
Pierre-Sassoulas
left a comment
There was a problem hiding this comment.
Thank you for the investigation. I wonder what are the performance implication and also if the side effect of being able to modify other reporter's message wasn't seen as a feature by some users. We'd need to do it as a breaking change if that's the case.
|
I guess copy works well with dataclass instances, Message contains only litterals. |
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
This comment has been minimized.
This comment has been minimized.
|
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit ece4392 |
|
We could also change the |
|
@
Do you have an usecase example ? Maybe that would be another option ? One issue is that we would need solve is to specify ordering. From 2.13.9 to 2.14.0, coloriezd/text reporter is now always the first reporter. If two reporters are necessary to achieve a specific output, maybe it should be only one reporter with options ? |
|
Yeah you're right. If anybody complains about this and has a usecase we can always update to allow it again. |
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Type of Changes
Description
MultiReporter was sending message by reference, which then could be changed by reporter. The colorized reporter was one of them, leading to changing any other reporters. This was broken since 2.14.0 and affected mostly people using colorized with a custom reporter.
Closes #7214