Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

.github: Add mergify configuration #3026

Merged
merged 4 commits into from
Nov 3, 2022
Merged

.github: Add mergify configuration #3026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
430dda5
Add mergify configuration
thomaseizinger Oct 14, 2022
f427d01
Add commit message body section to template
thomaseizinger Oct 27, 2022
6aba746
Merge branch 'master' into 3010-mergify
thomaseizinger Nov 2, 2022
06d8ad9
Merge branch 'master' into 3010-mergify
thomaseizinger Nov 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .github/mergify.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
defaults:
actions:
queue:
method: squash
commit_message_template: |
{{ title }} (#{{ number }})

{{ body | get_section("## Commit Message body", "") }}

pull_request_rules:
- name: Ask to resolve conflict
conditions:
- conflict
- -draft # Draft PRs are allowed to have conflicts.
actions:
comment:
message: This pull request has merge conflicts. Could you please resolve them @{{author}}? 🙏

- name: Add to merge queue
conditions:
# All branch protection rules are implicit: https://docs.mergify.com/conditions/#about-branch-protection
- label=send-it
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know how a PR is going to look like when all the checks have already passed but we didn't set this label yet? Will we be able to merge a PR manually in that case and skip the queue? Or does mergify add another required status check of their own to prevent that from happening?

I'm just curious, we can definitely merge and see. But if it is the case that the "normal" merges are still possible, we might want to think more about how we want to educate the users to use the queue instead of merging stuff directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you know how a PR is going to look like when all the checks have already passed but we didn't set this label yet? Will we be able to merge a PR manually in that case and skip the queue? Or does mergify add another required status check of their own to prevent that from happening?

Mergify only inherits the GitHub status checks like "Branch must be up to date". You'd be racing an automation if you try to merge it manually and I doubt you'd win :)

If the queue is currently empty, you can definitely merge manually.

I configured the squash commits such that those two scenarios should be indistuingishable.

If we pay for mergify premium, we can configure more queues where some have a higher priority.

I'm just curious, we can definitely merge and see. But if it is the case that the "normal" merges are still possible, we might want to think more about how we want to educate the users to use the queue instead of merging stuff directly.

Only maintainers can merge PRs so I am unsure who you are referring to with "users".

Copy link
Contributor

Choose a reason for hiding this comment

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

You'd be racing an automation if you try to merge it manually and I doubt you'd win :)

I wouldn't be so sure. Web hook deliveries are not instantaneous so there's definitely some headroom. Anyway, I was just curious how/if they addressed that. It doesn't necessarily have to be a problem. I'm definitely OK with checking it out in practice.

Only maintainers can merge PRs so I am unsure who you are referring to with "users".

Yes, I was referring to those with merge privileges. It's all the members of the teams listed in https://github.com/libp2p/github-mgmt/blob/246f37dac14efff89a5815b8de8ed5fbed2f4e14/github/libp2p.yml.

My thinking is that it'd be nice to have some sort of documentation for the maintainers available if we're changing the "PR merge" process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only maintainers can merge PRs so I am unsure who you are referring to with "users".

Yes, I was referring to those with merge privileges. It's all the members of the teams listed in libp2p/github-mgmt@246f37d/github/libp2p.yml.

My thinking is that it'd be nice to have some sort of documentation for the maintainers available if we're changing the "PR merge" process.

What is nice is that it doesn't fully change it but rather complement it. If no other PRs need to be merged and you have one that is up to date, you can just go and merge it, no need to use the queue!

Copy link
Contributor

Choose a reason for hiding this comment

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

But the queue itself is not standard. And because of that I think it requires some explanation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am gonna open an issue about this. At the moment we don't have any kind of documentation about our development processes, other than what is in the PR description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We kind of have an issue already: #2808

actions:
queue:

queue_rules:
- name: default
conditions: []
3 changes: 3 additions & 0 deletions .github/pull_request_template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] A changelog entry has been made in the appropriate crates

<!-- The below text will appear as the commit message body once we squash-merge the PR. -->
## Commit message body