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

ci: update mergify rules to include teams #2439

Merged
merged 2 commits into from
Aug 25, 2021
Merged

ci: update mergify rules to include teams #2439

merged 2 commits into from
Aug 25, 2021

Conversation

Madhu-1
Copy link
Collaborator

@Madhu-1 Madhu-1 commented Aug 25, 2021

updated mergify rules to consider the team's approval to merge a PR.

more details at #2367

fixes #2367

Signed-off-by: Madhu Rajanna [email protected]

@Madhu-1 Madhu-1 added the ci/skip/e2e skip running e2e CI jobs label Aug 25, 2021
@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 25, 2021

CC @ceph/ceph-csi-contributors @ceph/ceph-csi-maintainers

@mergify mergify bot added component/testing Additional test cases or CI work Repo activity Process/activites on ceph-csi repo labels Aug 25, 2021
nixpanic
nixpanic reviewed Aug 25, 2021
View reviewed changes
.mergify.yml
- "#changes-requested-reviews-by=0"
- "approved-reviews-by=@ceph/ceph-csi-contributors"
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if the notation should be @ceph-csi-contributors without the ceph/ organization part. Did you find an example configuration file where teams are used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

took example from Mergifyio/mergify#310 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess @ceph/ceph-csi-contributors is working
https://dashboard.mergify.io/github/ceph/config-editor?repositoryName=ceph-csi&pullRequestNumber=2425

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 25, 2021

This needs a manual merge. @humblec @nixpanic feel free to merge this PR after approval.

@mergify
Copy link
Contributor

mergify bot commented Aug 25, 2021

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

@Rakshith-R
Copy link
Contributor

@Madhu-1 , we need to preserve - "#approved-reviews-by>=2"
since a maintainer is also a contributor, its being counted twice, details in the link

https://dashboard.mergify.io/github/ceph/config-editor?repositoryName=ceph-csi&pullRequestNumber=2423

@mergify
Copy link
Contributor

mergify bot commented Aug 25, 2021

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 25, 2021

@Madhu-1 , we need to preserve - "#approved-reviews-by>=2"
since a maintainer is also a contributor, its being counted twice, details in the link

https://dashboard.mergify.io/github/ceph/config-editor?repositoryName=ceph-csi&pullRequestNumber=2423

Yes correct. updating it

@mergify
Copy link
Contributor

mergify bot commented Aug 25, 2021

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

@Madhu-1
ci: update mergify rules to include teams
edf64be 
updated mergify rules to consider the teams
approval to merge a PR.

more details at ceph#2367

fixes ceph#2367

Signed-off-by: Madhu Rajanna <[email protected]>
@humblec
Copy link
Collaborator

humblec commented Aug 25, 2021

@Madhu-1 i think its good to update the contribution guide with this too?

@mergify
Copy link
Contributor

mergify bot commented Aug 25, 2021

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

Rakshith-R
Rakshith-R reviewed Aug 25, 2021
View reviewed changes
Copy link
Contributor

@Rakshith-R Rakshith-R left a comment

Choose a reason for hiding this comment

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

Same changes missing for backport release v1.2 branch (line 105/107)but maybe remove backport pr rules for v1.2 completely too?
Otherwise LGTM.

@Madhu-1
doc: update development guide for new rules
66d1b6a 
updated development guide requirement to
have review from contributors and reviewers.

Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 25, 2021

@Madhu-1 i think its good to update the contribution guide with this too?

updated @humblec PTAL

@mergify
Copy link
Contributor

mergify bot commented Aug 25, 2021

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 25, 2021

Same changes missing for backport release v1.2 branch (line 105/107)but maybe remove backport pr rules for v1.2 completely too?
Otherwise LGTM.

v1.x is getting deprecated will remove all mergify rules for that in as a follow-up PR.

@mergify
Copy link
Contributor

mergify bot commented Aug 25, 2021

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

@humblec
Copy link
Collaborator

humblec commented Aug 25, 2021

@nixpanic are you good with the changes here?

@mergify
Copy link
Contributor

mergify bot commented Aug 25, 2021

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 25, 2021

Merging the PR as it has required approvals.

@Madhu-1 Madhu-1 merged commit b0b4668 into ceph:devel Aug 25, 2021
@mergify
Copy link
Contributor

mergify bot commented Aug 25, 2021

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Aug 25, 2021

Sorry but I didn't understand the command. Please consult the commands documentation .

Hey, I reacted but my real name is @Mergifyio

issue already reported for this at Mergifyio/mergify#3068

@mergify
Copy link
Contributor

mergify bot commented Aug 25, 2021

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nixpanic nixpanic nixpanic approved these changes

@Rakshith-R Rakshith-R Rakshith-R approved these changes

Assignees
No one assigned
Labels
ci/skip/e2e skip running e2e CI jobs component/testing Additional test cases or CI work Repo activity Process/activites on ceph-csi repo
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve contribution workflow to speed up issue tracking and PR merging
4 participants
@Madhu-1 @Rakshith-R @humblec @nixpanic