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

Allow merging when ready, regardless of predefined order #293

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nirizr
Copy link

@nirizr nirizr commented Feb 25, 2021

In this PR I propose two additional flags, mostly relevant to non-batch mode:

  1. ci-timeout-skip - Will change the behavior of the ci-timeout flag. The default behavior is that if a CI is running more than allowed by the ci-timeout flag, Marge will give up on merging that MR and will reassign it from itself. This flags allows that instead of unassigning Marge will just skip that MR and try again at a later time. This also prevents Marge from hanging on one MR for the CI's duration.
  2. skip-pending - This may not be the best name. Normally, Marge (in non-batch mode) will only attempt merging the first MR within a project (giving up as mentioned above if CI takes too long). This flag will attempt merging all relevant MRs, in the right order.

Our CI process takes an hour to run, and is under-resourced so some MR may take several hours to pass CI.
The combination of the above two flags allowed us to set a very short (1-5 minutes) ci-timeout and merge MRs as they become ready to be merged, instead of waiting a long while for a single MR within a project. I think this also allows Marge to better handle multiple projects in a parallel-like manner, but I'm not certain.

WDYT?

Disclaimers:

  1. I didn't yet check this PR but I've used something very similar for over 6 months now, I first wanna make sure the idea is acceptable and then will do what's required for it to be accepted.
  2. Until very recently we've used version 0.9.2, so this approach can conflict with features added after that version.
  3. I'm aware batch mode will also handle some of the issues that caused us to implement these flags but we rather avoid the batch mode for the time being.

merge_job.execute()
else:
for i, merge_request in enumerate(merge_requests):
log.info('Attempting to merge MR #%d / %d of assigned MRs...', i + 1, len(merge_requests))
Copy link

@okainov okainov Mar 15, 2021

Choose a reason for hiding this comment

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

Why not use enumerate(merge_requests, start=1) instead of using i+1? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants