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

Conversation

@bkchr
Copy link
Member

@bkchr bkchr commented Sep 18, 2020

No description provided.

@bkchr bkchr added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). 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 Sep 18, 2020
Copy link
Contributor

@andresilva andresilva left a comment

Choose a reason for hiding this comment

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

Won't this be a bit annoying? These are not the rules for a bot merge and in most cases we won't get 3 reviews on the companion.

@bkchr
Copy link
Member Author

bkchr commented Sep 18, 2020

Won't this be a bit annoying? These are not the rules for a bot merge and in most cases we won't get 3 reviews on the companion.

Not sure: paritytech/polkadot#1734 (comment)

Copy link
Contributor

@andresilva andresilva left a comment

Choose a reason for hiding this comment

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

I think the bot is enforcing github's approval threshold over its own rules, i.e. if some team lead approves the bot shouldn't care about having less than 3 reviews. We should fix that in the bot and this PR shouldn't be needed.

@bkchr
Copy link
Member Author

bkchr commented Sep 18, 2020

I think the bot is enforcing github's approval threshold over its own rules, i.e. if some team lead approves the bot shouldn't care about having less than 3 reviews. We should fix that in the bot and this PR shouldn't be needed.

While you are right, you are only half right.

People could start merging prs to Substrate if they have the required amount of approvals, while the companion doesn't have them and we end in the situation where we are currently. The Substrate pr was merged, but the polkadot pr can not yet be merged.

I think the solution to this problem should be that the process-bot ignores the check-polkadot-status and do it on its own. It could check if the companion was approved by a team lead and if yes, merge both prs.

@bkchr
Copy link
Member Author

bkchr commented Sep 29, 2020

@andresilva so what you think on how we want to continue here?

@bkchr bkchr closed this Oct 13, 2020
@bkchr bkchr deleted the bkchr-fix-companion-status branch October 13, 2020 08:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). 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

Development

Successfully merging this pull request may close these issues.

4 participants