Auto-merging of backporting PRs #3460
Replies: 3 comments 4 replies
-
I thought it using the https://github.blog/changelog/2021-02-04-pull-request-auto-merge-is-now-generally-available/ this feature which should for the merge requirements. Or could we merge, if requirements are not matched? I will later check our merge conditions. |
Beta Was this translation helpful? Give feedback.
-
I would vote for solution 3 if we can fix doc-only PRs as you suggested here (#3460 (reply in thread)). |
Beta Was this translation helpful? Give feedback.
-
@FlorianHockmann , @farodin91 I'm disabling this requirement:
I think it's unrealistic requirement for active branches. I.e. whenever someone merges anything we will need to trigger all branches to rebase and re-test. Tests may take quite some time and then the branch after being finished with testing may become outdated again and it will require re-testing again (i.e. endless process). For example, due to too many busy workers, the next PR #3268 was running tests for about 18 hours. After finishing with tests it again requires rebasing and testing against newest |
Beta Was this translation helpful? Give feedback.
-
#3297 enabled auto-merging of backporting PRs. My assumption was that this would only let the tool merge the PR after all actions were executed and all checks turned green. However, it looks like it only waits for required checks, but we currently don't require any checks for a PR to be merged. The reason why no checks are formally required is probably that we have a lot of flaky tests and some checks are prone for FPs like the test coverage or Codacy.
I would like to discuss here how we want to proceed with auto-merging of backporting PRs. I see three options:
Option 1 (disable auto-merge) means of course more manual work for us as we need to check the backporting PRs and merge them after all checks have passed.
Option 2 (auto-merge without checks) requires basically no effort from us. It could maybe also be argued that running all checks again on
v0.6
isn't necessary if they have already passed onmaster
. This is probably also similar to our previous merging strategy where we have mergedv0.6
intomaster
locally after merging a PR intov0.6
. Most of us probably also haven't executed all tests locally onmaster
before pushing the branch.Option 3 (add required checks & use them for auto-merge) seems like the safest option to me, but it changes also how we merge other PRs as they also need to pass all required checks. So, this requires more work from us.
After writing this, I personally tend towards option 3. I think it would help us in general to decide on which checks need to pass (like DCO, EasyCLA & all builds) and then motivate us to improve the situation with flaky tests. Flaky tests already slow us down a lot in my opinion (e.g. for Dependabot PR) so improving this helps us in general.
Auto-merging also makes sense in general from my side for backporting as we have already reviewed those changes for
master
so I don't see a point in reviewing them again forv0.6
.I would however definitely also be fine with option 1 or 2.
Any thoughts on this?
Beta Was this translation helpful? Give feedback.
All reactions