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

Turn on merge queues #143

Closed
2 of 9 tasks
surchs opened this issue Apr 15, 2024 · 6 comments
Closed
2 of 9 tasks

Turn on merge queues #143

surchs opened this issue Apr 15, 2024 · 6 comments
Assignees
Labels
process Improvement to internal processes

Comments

@surchs
Copy link
Contributor

surchs commented Apr 15, 2024

The overhead of dependabot PRs is mainly having to wait for them to go back to green after the previous one merged.
Github auto-merge can help, because it will automatically merge PRs once all requirements are met.

Tasks

Something to look into:

  • if I approve two bot PRs, does the CI for the second one run again after the first one was merged?
  • if I allow an external PR to auto-merge, and then the author makes accepts some code suggestions from me, do I have to re-enable the merge / merge manually

edit:
old and incomplete view:
That's what merge queues are for (and hopefully that works here to): https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue

So let's turn them on and let GitHub manage the merge sequence

@surchs surchs added process Improvement to internal processes quick fix Minimal planning and/or implementation work required. type:maintenance labels Apr 15, 2024
@surchs surchs added the flag:schedule Flag issue that should go on the roadmap or backlog. label Apr 15, 2024
@rmanaem rmanaem moved this to Backlog in Neurobagel Apr 17, 2024
@rmanaem rmanaem removed the flag:schedule Flag issue that should go on the roadmap or backlog. label Apr 17, 2024
@surchs surchs removed the quick fix Minimal planning and/or implementation work required. label Apr 22, 2024
@surchs
Copy link
Contributor Author

surchs commented May 1, 2024

I think I misunderstood this partly. What I am looking for is more "auto-merging": https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/automatically-merging-a-pull-request

Merge queues are also nice, but they address a problem that we don't have yet: how to merge a lot of PRs one after the other, if the CI for each PR takes a lot of time.

I think we can still enable merge queues if we want to (we now have them in https://github.com/neurobagel/react-bagel) but this issue should be about whether we want auto-merges turned on.

For us auto-merge would mean:

  • unless you explicitly disable it on your own PR, the PR will by default merge as soon as all checks are green, you are approved, and all other requirements are met

For external contributors it would mean:

  • we need to enable auto-merge for their PRs
  • and then the PR auto merges as well, unless they make another commit

@surchs surchs changed the title Turn on merge queues Turn on auto-merge for repos May 1, 2024
@surchs
Copy link
Contributor Author

surchs commented May 1, 2024

Some things we've learned already:

  • auto-merge runs once all checks and requirements are met
  • however: auto-merge does not rerun checks if the target branch has been pushed since the last check, and it also doesn't auto-merge / update PR branches

So for these things, you'd still want a merge queue

@surchs surchs moved this from Backlog to Specify - Active in Neurobagel May 8, 2024
@surchs surchs changed the title Turn on auto-merge for repos Turn on merge queues May 9, 2024
@surchs surchs moved this from Specify - Active to Specify - Done in Neurobagel May 9, 2024
@surchs
Copy link
Contributor Author

surchs commented May 9, 2024

@surchs surchs moved this from Specify - Done to Implement - Active in Neurobagel May 15, 2024
@surchs surchs self-assigned this May 15, 2024
@surchs
Copy link
Contributor Author

surchs commented May 15, 2024

Shared workflows we want to run on merge queues:

  • none (so far)

Repo specific workflows we want to run on merge queues

  • query
    • codespell (should be synced)
    • component test
    • e2e test -> these should be both in one workflow and they should use coverage like in our react template
    • lint
    • prettier -> they should both be in one workflow and also synced to all react repos!
    • codespell (see above)
  • n-api
    • codespell (see above)
    • test
  • f-API
    • codespell (see above)
    • lint -> should be synced to all python repos
    • test
  • cli
    • codespell (see above)
    • test

@surchs
Copy link
Contributor Author

surchs commented May 15, 2024

moving this issue to tracked until we address the following workflows issues:

Addressing these issues will also implicitly resolve the merge_queue issue but by making one change instead of 4 or 5

@surchs surchs moved this from Implement - Active to Implement - Track in Neurobagel May 15, 2024
@surchs surchs moved this from Implement - Track to Automation in Neurobagel May 18, 2024
@surchs surchs moved this from Automation to Implement - Track in Neurobagel May 21, 2024
@surchs
Copy link
Contributor Author

surchs commented Aug 2, 2024

Closing as we don't (yet) have very long running checks

@surchs surchs closed this as completed Aug 2, 2024
@github-project-automation github-project-automation bot moved this from Implement - Track to Review - Done in Neurobagel Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Improvement to internal processes
Projects
Archived in project
Development

No branches or pull requests

2 participants