search

teeworlds-community / teeworlds

A community fork of teeworlds. Trying to have more active maintainers and merge prs faster.
Other
11 stars 1 forks source link

How to handle passing CI requirement and outdated branches #99

Closed ChillerDragon closed 3 months ago

ChillerDragon commented 8 months ago

Problem no prs can be merged

The current teeworlds-community repository has all pipelines passing. And a merge queue active. The main branch is protected and only pull requests that pass all pipeline checks can be merged. As far as I know there is no way to configure github to only require that restriction after merge. Now the problem is that the pending prs are the exact same prs that are also pending upstream. And the upstream main branch has a failing pipeline. This means no pr passes the pipeline and thus can not be merged.

Possible ways to merge

Remove passing pipeline restriction

That sounds really bad. We need every tooling and safe guard we can get to ensure that this many people with merge powers do not mess up the main branch.

Require manual intervention from an org admin

That defeats the purpose of teeworlds-community. This project should not depend on some person with more power. If there is enough maintainers they should be able to merge anything without extra help.

Rebase pull requests before merge

The current pull requests are direct links from the upstream contributors github repos. So if they push to their contribution branch it is reflected instantly in teeworlds-community. That is fast and light weight. But the problem is that maintainers of teeworlds community can not write to those branches (including rebases). Also the authors of the pull requests can not really rebase either without breaking their upstream pull requests because it is the same branch. So the only way I see is copying the pull request. Opening a separate branch that can be written to by maintainers that has the same content as the upstream pr. That would be manual work. And it would also lose sync so if the upstream pr is being updated that is no longer instant and automatically reflected downstream. The manual work part of creating the copied pr I actually solved with mirror-bot but that comes with its own set of complexity and maintenance problems. Keeping it in sync could also be automated but it would introduce even more complexity and I am not sure if I find time to do that.

Anyways a sample of the first copy test can be seen here.

UPDATE: another sample where I was not the pr author https://github.com/teeworlds-community/teeworlds/pull/100. Here the problem is that I am being set as co author. Potentially merges help but that would then cause merge commits for every little patch.

Bamcane commented 8 months ago

I think maybe need the commit author to rebase their commit (if could). If couldn't (e.g. the commit author left teeworlds develop or too busy), we should to find a other way but idk what is it

ChillerDragon commented 8 months ago

I think maybe need the commit author to rebase their commit (if could). If couldn't (e.g. the commit author left teeworlds develop or too busy), we should to find a other way but idk what is it

That still would require the author to copy the pr and open a new pr just for teeworlds-community because it can not be rebased against teeworlds-community and be a upstream compatible pr at the same time.

jtbx commented 8 months ago

This is turning out to be a bit of a mess, considering whether it would be easier to just apply PRs as a patch and fix the conflicts manually

ChillerDragon commented 8 months ago

This is turning out to be a bit of a mess, considering whether it would be easier to just apply PRs as a patch and fix the conflicts manually

I think patches also mess with the author. Then all the commits would be marked co authored by some teeworlds community maintainer.

EDIT: I just saw your other comment saying it might not be github but git. Yea git has commiter and author fields maybe spoofing those solves that. I will give it a try :) nice idea!

ChillerDragon commented 8 months ago

Okay thanks to @jtbx I managed to do a rebase without merge commit and without touching any meta data so the original author is kept. See the example here https://github.com/teeworlds-community/teeworlds/pull/103

If the pr is not expected to change and be merged soon that seems like a smooth solution to me.

Until the mirror bot can also sync in updates I will leave it as reference by default. And only copy when we want to merge. This process of copying does not depend on me being there. Anyone can run the mirror bot to copy a pr or even manually copy a pr.

It is not perfect but I would say it is good enough for now to move forward.

jtbx commented 8 months ago

Cool cool thanks a lot. Yeah it's not perfect but git makes it pretty easy to change history so that's always an option. However, I was trying to push a fix after I rebased #100 to revert the committer identity and I got an "access denied" error trying to push to the mirror's branch which kind of defeats the point of that function unless you want to use github's web UI :smile:. Is that some sort of setting in the repo?

ChillerDragon commented 8 months ago

Cool cool thanks a lot. Yeah it's not perfect but git makes it pretty easy to change history so that's always an option. However, I was trying to push a fix after I rebased #100 to revert the committer identity and I got an "access denied" error trying to push to the mirror's branch which kind of defeats the point of that function unless you want to use github's web UI 😄. Is that some sort of setting in the repo?

Not sure what happend here. It should have allow maintainer edits on. Lets hope it was a hiccup because I deleted the branch during that time? :shrug:

I just cba to do further debugging for that now. Since you are the only person so far who wanted to do that I invited you to https://github.com/teeworlds-mirror/teeworlds_teeworlds-mirror-prs/ so that should hot fix it hopefully.

ChillerDragon commented 3 months ago

Copying the branches doing a rebase and spoofing the original author seems like a fine solution. The mirror bot script is capable of doing it and so far it has been smooth.

Closing this as completed.