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

Revert "sc-cli: add no-beefy flag to cli config (#14754)" - #14766

Merged
paritytech-processbot[bot] merged 1 commit into
paritytech:masterfrom
acatangiu:sc-cli-beefy-flag
Aug 15, 2023
Merged

Revert "sc-cli: add no-beefy flag to cli config (#14754)"#14766
paritytech-processbot[bot] merged 1 commit into
paritytech:masterfrom
acatangiu:sc-cli-beefy-flag

Conversation

@acatangiu

@acatangiuacatangiu commented Aug 14, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

This reverts commit 2060c36.

Based on #14754 (comment)

polkadot companion: paritytech/polkadot#7613
cumulus companion: paritytech/cumulus#3007

@acatangiuacatangiu 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 14, 2023
@acatangiuacatangiu self-assigned this Aug 14, 2023
This was referenced Aug 14, 2023
Merged
Merged
@acatangiu

acatangiu commented Aug 14, 2023

Copy link
Copy Markdown
ContributorAuthor

@bkchr

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

shall I also remove disable_grandpa then (remove --no-grandpa from sc-cli)? If yes, will do it as separate PR so that this is clean revert and nothing else.

@bkchr

bkchr commented Aug 15, 2023

Copy link
Copy Markdown
Member

@bkchr

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

shall I also remove disable_grandpa then (remove --no-grandpa from sc-cli)? If yes, will do it as separate PR so that this is clean revert and nothing else.

Sounds like a good idea! WDYT @andresilva?

@acatangiu

acatangiu commented Aug 15, 2023

Copy link
Copy Markdown
ContributorAuthor

bot merge

@paritytech-processbot

paritytech-processbotBot commented Aug 15, 2023

Copy link
Copy Markdown

Error: Github API says paritytech/polkadot#7613 is not mergeable

@andresilva

andresilva commented Aug 15, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

The disable_grandpa flag is used in both the node and node-template services, those are just using directly the RunCmd from sc-cli and it's probably not worth it to create their own RunCmd structs just for this. I'm fine with removing it, but it should still be added to polkadot IMO.

@acatangiu

acatangiu commented Aug 15, 2023

Copy link
Copy Markdown
ContributorAuthor

bot merge

@paritytech-processbot
paritytech-processbotBot merged commit 8a2c265 into paritytech:masterAug 15, 2023
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 freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

4 more reviewers
@andresilvaandresilvaandresilva approved these changes
@bkchrbkchrbkchr approved these changes
@serban300serban300serban300 approved these changes
@lexnvlexnvlexnv approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@acatangiuacatangiu

Labels

A0-please_reviewPull request needs code review.B0-silentChanges should not be mentioned in any release notesC1-lowPR 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.

5 participants

@acatangiu@bkchr@andresilva@serban300@lexnv