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

sc-cli: add no-beefy flag to cli config #14754

Merged
merged 1 commit into from
Aug 11, 2023
Merged

sc-cli: add no-beefy flag to cli config #14754

merged 1 commit into from
Aug 11, 2023

Conversation

acatangiu
Copy link
Contributor

@acatangiu acatangiu commented Aug 11, 2023 •
edited
Loading

BEEFY is soon expected to run on all substrate-based chains, make it part of sc-cli.

Note: in case of polkadot binary, CLI doesn't change. --no-beefy is moved from polkadot specific cli to sc-cli.

polkadot companion: paritytech/polkadot#7600
cumulus companion: paritytech/cumulus#2996

@acatangiu acatangiu added 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. labels Aug 11, 2023
@acatangiu acatangiu self-assigned this Aug 11, 2023
@acatangiu acatangiu mentioned this pull request Aug 11, 2023
Merged
@acatangiu acatangiu requested a review from andresilva August 11, 2023 11:16
lexnv
lexnv approved these changes Aug 11, 2023
View reviewed changes
Copy link
Contributor

@lexnv lexnv left a comment

Choose a reason for hiding this comment

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

LGTM! I may also need a cumulus companion

@acatangiu acatangiu mentioned this pull request Aug 11, 2023
Merged
@acatangiu
Copy link
Contributor Author

LGTM! I may also need a cumulus companion

You're right! done.

@acatangiu
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 2060c36 into paritytech:master Aug 11, 2023
@bkchr
Copy link
Member

bkchr commented Aug 14, 2023

BEEFY is soon expected to run on all substrate-based chains

That is a bold statement that isn't true...

I know that we also have there disable_grandpa, that is just some old oversight and we should not replicate it.

@acatangiu
Copy link
Contributor Author

BEEFY is soon expected to run on all substrate-based chains

That is a bold statement that isn't true...

I hope it will prove true 😛

I know that we also have there disable_grandpa, that is just some old oversight and we should not replicate it.

So you suggest we revert to having --no-beefy as a polkadot-specific CLI flag as opposed to sc-cli flag?

@bkchr
Copy link
Member

bkchr commented Aug 14, 2023

So you suggest we revert to having --no-beefy as a polkadot-specific CLI flag as opposed to sc-cli flag?

Yes please. Not all Substrate chains are running grandpa either and the same will apply to beefy.

acatangiu added a commit to acatangiu/substrate that referenced this pull request Aug 14, 2023
@acatangiu
Revert "sc-cli: add no-beefy flag to cli config (paritytech#14754)"
d6368cd 
This reverts commit 2060c36.
This was referenced Aug 14, 2023
Merged
Merged
Merged
paritytech-processbot bot pushed a commit that referenced this pull request Aug 15, 2023
@acatangiu
Revert "sc-cli: add no-beefy flag to cli config (#14754)" (#14766)
8a2c265 
This reverts commit 2060c36.
Ank4n pushed a commit that referenced this pull request Aug 20, 2023
@acatangiu @Ank4n
sc-cli: add no-beefy flag to cli config (#14754)
307cec8 
Signed-off-by: Adrian Catangiu <[email protected]>
Ank4n pushed a commit that referenced this pull request Aug 20, 2023
@acatangiu @Ank4n
Revert "sc-cli: add no-beefy flag to cli config (#14754)" (#14766)
28afcb8 
This reverts commit 2060c36.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lexnv lexnv lexnv approved these changes

@andresilva andresilva andresilva approved these changes

Assignees

@acatangiu acatangiu

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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@acatangiu @bkchr @andresilva @lexnv