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

Try-state for Referenda pallet #13949

Merged
merged 32 commits into from
Jul 30, 2023
Merged

Try-state for Referenda pallet #13949

merged 32 commits into from
Jul 30, 2023
+145 −41

Conversation

Szegoo
Copy link
Contributor

@Szegoo Szegoo commented Apr 18, 2023

@Szegoo Szegoo marked this pull request as ready for review April 22, 2023 18:26
Szegoo
Szegoo commented Apr 22, 2023
View reviewed changes
frame/referenda/src/lib.rs Outdated Show resolved Hide resolved
Szegoo
Szegoo commented Apr 22, 2023
View reviewed changes
frame/referenda/src/lib.rs Outdated Show resolved Hide resolved
@Szegoo
Copy link
Contributor Author

Szegoo commented Apr 23, 2023

@ggwpez Could you take a quick look?

muharem
muharem reviewed Apr 25, 2023
View reviewed changes
frame/referenda/src/lib.rs Outdated Show resolved Hide resolved
frame/referenda/src/lib.rs Outdated Show resolved Hide resolved
frame/referenda/src/lib.rs Outdated Show resolved Hide resolved
frame/referenda/src/lib.rs Outdated Show resolved Hide resolved
frame/referenda/src/lib.rs Outdated
/// * The submission deposit cannot be less than `T::SubmissionDeposit`.
///
/// Looking at tracks:
/// * The `TrackQueue` should be empty if `DecidingCount` is less than
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is possible when something is removed from the decision stage, and only with the next block the task from the queue will be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case, this comment should probably be updated:

substrate/frame/referenda/src/lib.rs

Lines 238 to 240 in 6f0f5a9

/// This should be empty if `DecidingCount` is less than `TrackInfo::max_deciding`.
#[pallet::storage]
pub type TrackQueue<T: Config<I>, I: 'static = ()> = StorageMap<

Copy link
Contributor

Choose a reason for hiding this comment

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

If in doubt, please remove it form the PR so this can be moved forward.

frame/referenda/src/lib.rs Outdated Show resolved Hide resolved
frame/referenda/src/lib.rs Outdated Show resolved Hide resolved
frame/referenda/src/migration.rs Outdated Show resolved Hide resolved
Szegoo and others added 2 commits April 25, 2023 15:59
@Szegoo
Copy link
Contributor Author

Szegoo commented Apr 25, 2023 •
edited
Loading

@muharem Thanks for your review, I am moving this into draft now, I want to give this some more thought to see if there are some other things that should be added/removed.

@Szegoo Szegoo marked this pull request as draft April 25, 2023 14:08
@Szegoo Szegoo marked this pull request as ready for review April 30, 2023 07:19
@Szegoo Szegoo requested a review from muharem May 6, 2023 16:24
@juangirini juangirini added the T1-runtime This PR/Issue is related to the topic “runtime”. label May 22, 2023
@kianenigma kianenigma mentioned this pull request May 22, 2023
Open
62 tasks
@stale
Copy link

stale bot commented Jun 21, 2023

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A3-stale label Jun 21, 2023
@Szegoo Szegoo requested review from a team June 21, 2023 10:49
@stale stale bot removed A3-stale labels Jun 21, 2023
@liamaharon liamaharon added the C1-low PR touches the given topic and has a low impact on builders. label Jul 7, 2023
@liamaharon
Copy link
Contributor

can you please rebase?

liamaharon
liamaharon reviewed Jul 28, 2023
View reviewed changes
frame/referenda/src/migration.rs Outdated Show resolved Hide resolved
frame/referenda/src/mock.rs Outdated Show resolved Hide resolved
frame/referenda/src/lib.rs Outdated Show resolved Hide resolved
@liamaharon liamaharon added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label Jul 28, 2023
@gpestana
Copy link
Contributor

bot fmt

@command-bot
Copy link

command-bot bot commented Jul 28, 2023 •
edited
Loading

@gpestana https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3288286 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 32-0c48ce11-48c5-4750-a1bc-7885358defc8 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jul 28, 2023

@gpestana Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3288286 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3288286/artifacts/download.

gpestana
gpestana approved these changes Jul 28, 2023
View reviewed changes
Copy link
Contributor

@gpestana gpestana left a comment

Choose a reason for hiding this comment

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

Left a comment but lgtm, thanks for the PR!

frame/referenda/src/lib.rs Show resolved Hide resolved
@liamaharon
Copy link
Contributor

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 31491fa into paritytech:master Jul 30, 2023
8 checks passed
@liamaharon
Copy link
Contributor

/tip medium

@substrate-tip-bot
Copy link

@liamaharon You are not allowed to request a tip. Only members of paritytech/tip-bot-approvers are allowed.

@liamaharon
Copy link
Contributor

liamaharon commented Jul 31, 2023 •
edited
Loading

@juangirini could you please tip?

@juangirini
Copy link
Contributor

@Szegoo could you please add your dot/ksm address to the PR description for a tip
format "[Polkadot|Kusama] address: xxx"

@rzadp
Copy link
Contributor

rzadp commented Aug 2, 2023 •
edited
Loading

@juangirini Recent release of the tip bot can now Read the contributor address from their github profile (bio).

@Szegoo Has the address in the bio so we can try that.

@juangirini
Copy link
Contributor

@rzadp that's awesome, thanks for the update

@juangirini
Copy link
Contributor

/tip medium

@substrate-tip-bot
Copy link

@juangirini A medium (80 DOT) tip was successfully submitted for @Szegoo (126X27SbhrV19mBFawys3ovkyBS87SGfYwtwa8J2FjHrtbmA on polkadot).

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Frpc.polkadot.io#/referenda tip

@crystalin crystalin mentioned this pull request Oct 20, 2023
Closed
@JesseAbram JesseAbram mentioned this pull request Oct 26, 2023
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gpestana gpestana gpestana approved these changes

@kianenigma kianenigma kianenigma approved these changes

@liamaharon liamaharon liamaharon approved these changes

@muharem muharem Awaiting requested review from muharem

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Szegoo @liamaharon @gpestana @juangirini @rzadp @kianenigma @muharem