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.

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

Add mergify configuration #141

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

okurz
Copy link
Member

@okurz okurz commented Oct 12, 2021

No description provided.

@Martchus
Copy link
Contributor

Looks like zypper crashes due to an unhandled C++ exception:

terminate called after throwing an instance of 'std::system_error'
  what():  Operation not permitted

I suppose this could be considered a zypper bug.

@okurz
Copy link
Member Author

okurz commented Oct 12, 2021

@okurz
Copy link
Member Author

okurz commented Oct 13, 2021

meh, "Package 'python3-black' not found." in Leap 15.3

@Martchus
Copy link
Contributor

Right, I only added it recently to devel:openQA:Leap:15.3 but that's of course not helping here. We needed a container with our devel repo enabled, maybe just reuse the openQA CI container?

@okurz
Copy link
Member Author

okurz commented Oct 13, 2021

to be fixed in #142

@okurz okurz marked this pull request as draft December 3, 2022 18:00
@okurz okurz marked this pull request as ready for review July 25, 2024 19:32
@okurz
Copy link
Member Author

okurz commented Jul 25, 2024

Rebased, refreshed, updated, verified. All ready for review now.

Copy link
Member

@foursixnine foursixnine left a comment

Choose a reason for hiding this comment

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

Change the approval requirement to members of @os-autoinst/asset-sync-and-integration

@okurz
Copy link
Member Author

okurz commented Jul 26, 2024

Change the approval requirement to members of @os-autoinst/asset-sync-and-integration

I don't think that's possible according to https://docs.mergify.com/workflow/actions/merge/ . Also I don't think that we need to overcomplicate things. Still only approvals from priviledged members of the organisation count, not like anybody

@foursixnine
Copy link
Member

foursixnine commented Jul 27, 2024

Change the approval requirement to members of @os-autoinst/asset-sync-and-integration

I don't think that's possible according to https://docs.mergify.com/workflow/actions/merge/ . Also I don't think that we need to overcomplicate things. Still only approvals from priviledged members of the organisation count, not like anybody

Not convinced. Ever heard of bad actors and account takeover? - If others (non members of @os-autoinst/tools-team) agree, and number of approvals is increased to 2 and I can reconsider.

I am not a fan of automated merges on things that go live - also no description is provided nor reasoning as to why is it important for this repository.

@foursixnine foursixnine added the do not merge Proposed changes that require further discussion label Jul 27, 2024
@okurz
Copy link
Member Author

okurz commented Jul 27, 2024

Change the approval requirement to members of @os-autoinst/asset-sync-and-integration

I don't think that's possible according to https://docs.mergify.com/workflow/actions/merge/ . Also I don't think that we need to overcomplicate things. Still only approvals from priviledged members of the organisation count, not like anybody

Not convinced. Ever heard of bad actors and account takeover? - If others (non members of @os-autoinst/tools-team) agree, and number of approvals is increased to 2 and I can reconsider.

Sure, let's play it safe(r). Increased to >=2 now.

I am not a fan of automated merges on things that go live - also no description is provided nor reasoning as to why is it important for this repository.

Well, first it might seem to be more convenient than it is important but we have good experiences in os-autoinst, openQA and others and the approach is helping to improve the quality and prevent unforeseen problems because unlike human reviewers mergify is very diligent to ensure that all necessary CI checks are not failed but also present as happened multiple times that CI checks were not triggered at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Proposed changes that require further discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants