On Wed, Jan 22, 2020 at 11:24 PM David Greene <greened at obbligato.org> wrote:> Christian Kühnel via llvm-dev <llvm-dev at lists.llvm.org> writes: > > >>> In Github pull requests there is always a git commit that you can just > >>> feed to the build server. And you can be sure of what really gets > merged. > >>> You review, build and test exactly the change that gets merged > afterwards. > >>> > >> > >> How would that be true? Given that upstream keep changing during the > >> period of review? The commit is going to have to be rebased to head and > >> that may involve making changes. > >> > > > > Yes, github tells you, that your PR has a merge conflict, that you need > to > > resolve manually. Once you've pushed the conflict resolution to the same > > PR, it triggers another round of builds and tests. And also potentially > > another review, depending on what permissions you have and how the > project > > ist set up... > > It's not quite that simple though. In general most PRs will need to be > rebased just before merging to maintain linear history.That depends on how frequent merge conflicts are. In my other projects that wasn't really an issue. I rarely had to rebase something manually.> What happens > then? Would things need to pass another build before the "final merge?" >If so, then something needs to keep a list of pending PRs waiting for> "final merge." This quickly gets very hairy as pending PRs have to > constantly be rebased and rebuilt as other PRs land. >Some projects have a flag they set and let some bot do the manual work: Rebase, re-test and then automatically merge. I do not expect this to be perfect, it should rather be much better than today in slipping new bugs into master. -- Best, Christian -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200123/51ab2ff5/attachment-0001.html>
Christian Kühnel <kuhnel at google.com> writes:>> It's not quite that simple though. In general most PRs will need to be >> rebased just before merging to maintain linear history. > > > That depends on how frequent merge conflicts are. In my other projects that > wasn't really an issue. I rarely had to rebase something manually.It has little to do with merge conflicts. Because LLVM requires fast-forward merges, everything has to be rebased after something else gets merged.>> What happens then? Would things need to pass another build before >> the "final merge?" >> >> If so, then something needs to keep a list of pending PRs waiting for >> "final merge." This quickly gets very hairy as pending PRs have to >> constantly be rebased and rebuilt as other PRs land. > > Some projects have a flag they set and let some bot do the manual work: > Rebase, re-test and then automatically merge. > I do not expect this to be perfect, it should rather be much better than > today in slipping new bugs into master.I agree the process can be improved. But at some point builders are going to be backed up as each PR gets rebased and then rebuilt just before merging. Probably we will have to punt an assume that if the rebase is clean, it can just be merged in without another build. We will miss some small cases where non-conflicting code interacts in bad ways but as you said, no system is perfect. -David
On Thu, Jan 23, 2020 at 5:47 PM David Greene <greened at obbligato.org> wrote:> Christian Kühnel <kuhnel at google.com> writes: > > >> It's not quite that simple though. In general most PRs will need to be > >> rebased just before merging to maintain linear history. > > > > > > That depends on how frequent merge conflicts are. In my other projects > that > > wasn't really an issue. I rarely had to rebase something manually. > > It has little to do with merge conflicts. Because LLVM requires > fast-forward merges, everything has to be rebased after something else > gets merged. >This is done by github PR automagically as long as there are no merge conflicts.> > >> What happens then? Would things need to pass another build before > >> the "final merge?" > >> > >> If so, then something needs to keep a list of pending PRs waiting for > >> "final merge." This quickly gets very hairy as pending PRs have to > >> constantly be rebased and rebuilt as other PRs land. > > > > Some projects have a flag they set and let some bot do the manual work: > > Rebase, re-test and then automatically merge. > > I do not expect this to be perfect, it should rather be much better than > > today in slipping new bugs into master. > > I agree the process can be improved. But at some point builders are > going to be backed up as each PR gets rebased and then rebuilt just > before merging. Probably we will have to punt an assume that if the > rebase is clean, it can just be merged in without another build. We > will miss some small cases where non-conflicting code interacts in bad > ways but as you said, no system is perfect. >Yes, I agree. -- Best, Christian -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200123/bde5894a/attachment.html>