Restore allWarningsAsErrors in main source-sets #4336
Conversation
|
Let's merge it and with the next step I'll enable it in the dev builds |
| @@ -64,7 +66,7 @@ public actual fun <T> runBlocking(context: CoroutineContext, block: suspend Coro | |||
| var completed = false | |||
| ThreadLocalKeepAlive.addCheck { !completed } | |||
| try { | |||
| @Suppress("LEAKED_IN_PLACE_LAMBDA") // Contract is preserved, invoked immediately or throws | |||
|
|
|||
There was a problem hiding this comment.
Is this newline a typo?
| @@ -316,6 +316,7 @@ public class PublisherCoroutine<in T>( | |||
| signalCompleted(cause, handled) | |||
| } | |||
|
|
|||
| @Deprecated("Since 1.2.0, binary compatibility with versions <= 1.1.x", level = DeprecationLevel.HIDDEN) | |||
There was a problem hiding this comment.
Binary compatibility is not the only reason why we have this method. If we remove it, the Subscription interface is no longer implemented. In fact, I'd argue it's a bug that this code causes a warning: https://youtrack.jetbrains.com/issue/KT-74697/Overriding-a-method-thats-both-deprecated-and-non-deprecated-should-not-cause-warnings
There was a problem hiding this comment.
Nice catch!
I propose to leave it as is here because for users everything keeps working (i.e. PublisherCoroutine,cancel is not exposed) and we do not provide any new documentation here
There was a problem hiding this comment.
I don't object to @Deprecated, as long as the error message honestly states the reason and doesn't mislead.
There was a problem hiding this comment.
Now that I think about it more, @Deprecated is slightly worse than a suppression, because if https://youtrack.jetbrains.com/issue/KT-74697 does get fixed, we'll be notified about it via the "redundant suppression" message, whereas the deprecation is with us to stay.
| @@ -316,6 +316,7 @@ public class PublisherCoroutine<in T>( | |||
| signalCompleted(cause, handled) | |||
| } | |||
|
|
|||
| @Deprecated("Since 1.2.0, binary compatibility with versions <= 1.1.x", level = DeprecationLevel.HIDDEN) | |||
There was a problem hiding this comment.
Now that I think about it more, @Deprecated is slightly worse than a suppression, because if https://youtrack.jetbrains.com/issue/KT-74697 does get fixed, we'll be notified about it via the "redundant suppression" message, whereas the deprecation is with us to stay.
No description provided.