Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

reschedule #6860

Merged
9 commits merged into from
Oct 7, 2020
Merged

reschedule #6860

9 commits merged into from
Oct 7, 2020

Conversation

xlc
Copy link
Contributor

@xlc xlc commented Aug 9, 2020

Fixes #6774

Did not add dispatchable calls because we don't need it. Happy to add it if people wants it.

TODOs:

  • tests

@xlc xlc added A0-please_review Pull request needs code review. B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. labels Aug 9, 2020
Copy link
Contributor

@danforbes danforbes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few documentation nitpicks

frame/scheduler/src/lib.rs Outdated Show resolved Hide resolved
frame/support/src/traits.rs Outdated Show resolved Hide resolved
frame/support/src/traits.rs Outdated Show resolved Hide resolved
Co-authored-by: Dan Forbes <dan@danforbes.dev>
Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Ok(())
})?;

let new_index = Agenda::<T>::decode_len(new_time).unwrap_or(1) as u32 - 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its more efficient to get the length from the try_mutate above right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agenda::<T>::append was used which doesn't decode the whole thing, it does have the length info but not exposed

Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rescheduling to same time ends up removing the task: https://github.com/paritytech/substrate/pull/6860/files#r470499784 (fixed)

@gui1117 gui1117 self-requested a review August 15, 2020 09:19
@gui1117 gui1117 dismissed their stale review August 15, 2020 09:19

requested changes is fixed

@xlc
Copy link
Contributor Author

xlc commented Aug 19, 2020

Good to merge or still need approval from @gavofyork ?

@gui1117
Copy link
Contributor

gui1117 commented Aug 19, 2020

should this needs audit ?

@gavofyork gavofyork added the D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited label Aug 19, 2020
@gavofyork
Copy link
Member

gavofyork commented Aug 19, 2020

yes. at this point anything that touches the polkadot runtime needs an audit.

@gnunicorn gnunicorn added A8-mergeoncegreen and removed A0-please_review Pull request needs code review. A8-mergeoncegreen labels Sep 9, 2020
@xlc
Copy link
Contributor Author

xlc commented Sep 23, 2020

Any updates?

@shawntabrizi
Copy link
Member

It is in queue for being Audited, but not yet complete.

@viniul
Copy link
Contributor

viniul commented Oct 7, 2020

We are done with the review for this PR - looks good to us!

@shawntabrizi shawntabrizi added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Oct 7, 2020
@shawntabrizi
Copy link
Member

bot merge

@ghost
Copy link

ghost commented Oct 7, 2020

Trying merge.

@ghost ghost merged commit 5775849 into paritytech:master Oct 7, 2020
@shawntabrizi shawntabrizi deleted the reschedule branch October 7, 2020 14:51
ordian added a commit that referenced this pull request Oct 9, 2020
…up-updates

* master:
  Async keystore + Authority-Discovery async/await (#7000)
  Fixes logging of target names with dashes (#7281)
  seal: Add automated weights for contract API calls (#7017)
  add ss58 id for nodle (#7279)
  Refactor CurrencyToVote (#6896)
  bump-allocator: document & poison (#7277)
  Reset flaming fir network (#7274)
  reschedule (#6860)
  Drop system cache for trie benchmarks (#7242)
  client: improve log formatting (#7272)
  Rework `InspectState` (#7271)
  SystemOrigin trait (#7226)
  Update ss58 registry for Dock network (#7263)
  .maintain/monitoring: Add alert when continuous task ends (#7250)
  Rename `TRANSACTION_VERSION` to `EXTRINSIC_VERSION` (#7258)
  Split block announce processing into two parts (#6958)
  Fix offchain election to respect the weight (#7215)
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited
Projects
None yet
Development

Successfully merging this pull request may close these issues.

reschedule a scheduled task
8 participants