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

Decouple Stkaing and Election - Part1: Support traits #7908

Merged
5 commits merged into from
Jan 18, 2021
Merged

Decouple Stkaing and Election - Part1: Support traits #7908

5 commits merged into from
Jan 18, 2021

Conversation

kianenigma
Copy link
Contributor

@kianenigma kianenigma commented Jan 15, 2021 •
edited
Loading

Attempt to break down #7319. Part 1

Part 2: #7909
Part 3: #7910

Only contains the support traits.

@kianenigma kianenigma added A0-please_review Pull request needs code review. B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. labels Jan 15, 2021
This was referenced Jan 15, 2021
Merged
Merged
gui1117
gui1117 approved these changes Jan 15, 2021
View reviewed changes
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.

apart from above nitpicks, looks good to me

@gui1117
Copy link
Contributor

gui1117 commented Jan 15, 2021

but we intent to make this PR compiles and merge into master no ?
otherwise splitting isn't very valuable.

@kianenigma
Copy link
Contributor Author

I guess it is a good idea to merge this and rebase the other two, if I can get it to compile.

@shawntabrizi
Copy link
Member

yeah that would be really nice

@gui1117
Copy link
Contributor

gui1117 commented Jan 16, 2021

I guess it is a good idea to merge this and rebase the other two, if I can get it to compile.

At some point I would really prefer this. Maybe I'm bad at reviewing PR's but IMO reviewing is not linear in the number of changes or adds of the PR.
I think like splitting PR doesn't take much time comparing the benefit for catching errors.

@kianenigma
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Jan 18, 2021

Trying merge.

@ghost ghost merged commit 4ac74e9 into master Jan 18, 2021
@ghost ghost deleted the kiz-election-provider-1-base branch January 18, 2021 10:24
@kianenigma kianenigma mentioned this pull request Aug 24, 2023
Open
This was referenced Feb 11, 2021
Closed
Merged
@jordy25519 jordy25519 mentioned this pull request Feb 22, 2022
Open
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gui1117 gui1117 gui1117 approved these changes

@shawntabrizi shawntabrizi shawntabrizi approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kianenigma @gui1117 @shawntabrizi @paritytech-ci