-
Notifications
You must be signed in to change notification settings - Fork 398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DO NOT REVIEW] feat: update to polkadot v1.1.0 release #1157
Conversation
run_to_block::<T>(state.next_era_start - 1); | ||
DappStaking::<T>::on_finalize(state.next_era_start - 1); | ||
System::<T>::set_block_number(state.next_era_start); | ||
run_to_block::<T>((state.next_era_start - 1).into()); |
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.
Perhaps the helper functions should do this conversion?
E.g. change function argument to u64
.
Seems like could would be much more readable.
pub(super) fn run_to_block<T: Config>(n: BlockNumberFor<T>) | ||
where | ||
BlockNumberFor<T>: IsType<BlockNumber>, | ||
{ |
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.
Perhaps it can be solved by my previous comment, but we should really consider defining the custom type to be sued here, to avoid repeating this lengthy type description everywhere.
#[benchmarks( | ||
where | ||
BlockNumberFor<T>: IsType<BlockNumber>, | ||
)] |
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.
A general comment about this, since it's repeating really a lot - astar-primitives
were introduced to avoid having such code snippets, to cut down on the code repetition. And now we find ourselves going in the separate direction 🙂.
Should we get rid of the BlockNumber
from the astar-primitives
?
Should we instead define our custom type, which would cover the new BlockNumberFor
requirements?
We'd all prefer I guess not to have to repeat the where
statement everywhere.
Some(Subcommand::TryRuntime) => Err("The `try-runtime` subcommand has been migrated to a \ | ||
standalone CLI (https://github.com/paritytech/try-runtime-cli). It is no longer \ | ||
being maintained here and will be removed entirely some time after January 2024. \ | ||
Please remove this subcommand from your runtime and use the standalone CLI." | ||
.into()), |
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.
Why not just remove it now?
Pull Request Summary
This PR has been superseded by #1182