-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Conversation
Cumulus CI is stuck in paritytech/cumulus#2574, so companion check will be red. Changes_ - Controller not needed Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@@ -631,7 +631,7 @@ pub mod pallet { | |||
MaxNominatorsCount::<T>::put(x); | |||
} | |||
|
|||
for &(ref stash, ref controller, balance, ref status) in &self.stakers { | |||
for &(ref stash, _, balance, ref status) in &self.stakers { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we don't need controller
as part of stakers
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but that would require companions in Polkadot/Cumulus, and the CI checks are already red.
So @paritytech/staking-core could do this as follow-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or I do it and you force-merge it 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need prs in Polkadot and cumulus any was to fix ci?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont hope so. The checks in paritytech/cumulus#2574 should go green once this is merged, and unless Polkadot turns red in the companion check here it should be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests in Cumulus fail without this change, as you can see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rossbulat is there any follow up on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I amended 'stakers' here: paritytech/polkadot#7224
Are the cumulus tests working now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I meant to remove controllers
from the Vec
entirely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue has been opened.
The most important thing was to put the plug on new unique pairs initially. We now have time to tidy up the code in follow ups.
bot merge |
Cumulus CI is stuck in paritytech/cumulus#2574, so companion check will be red. Changes_ - Controller not needed Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Changes:
Fixing the Cumulus CI from paritytech/cumulus#2574 - companion checks will be red.
Cumulus companion: paritytech/cumulus#2574