You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In #2785, we added a PendingUpgrade implementation that is essentially equivalent to DeniedUpgrade apart from also needing a protocol. @mxinden was this on purpose or an oversight? DeniedUpgrade has quite some usage throughout the codebase but I think I prefer the name PendingUpgrade as it reflects the use of futures::pending. At the same time, passing a protocol to PendingUpgrade is unnecessary IMO.
What do people think of removing
Motivation
Having two upgrades with the same functionality is unnecessary.
Current Implementation
Are you planning to do it yourself in a pull request?
Maybe.
The text was updated successfully, but these errors were encountered:
In #2785, we added a PendingUpgrade implementation that is essentially equivalent to DeniedUpgrade apart from also needing a protocol. @mxinden was this on purpose or an oversight?
They serve two different purposes. PendingUpgrade provides a protocol name during multistream-select negotiation, but never proceeds once the protocol has been negotiated. DeniedUpgrade provides no protocol name during multistream-select negotiation, thereby the multistream-select negotiation fails.
I can not think of a use-case for PendingUpgrade beyond testing, though I think that is enough to justify its existence.
Description
In #2785, we added a
PendingUpgrade
implementation that is essentially equivalent toDeniedUpgrade
apart from also needing a protocol. @mxinden was this on purpose or an oversight?DeniedUpgrade
has quite some usage throughout the codebase but I think I prefer the namePendingUpgrade
as it reflects the use offutures::pending
. At the same time, passing a protocol toPendingUpgrade
is unnecessary IMO.What do people think of removing
Motivation
Having two upgrades with the same functionality is unnecessary.
Current Implementation
Are you planning to do it yourself in a pull request?
Maybe.
The text was updated successfully, but these errors were encountered: