> On 7. 7. 2022, at 19:28, Nir Soffer <nsof...@redhat.com> wrote: > > On Wed, Jun 15, 2022 at 12:26 PM Yedidyah Bar David <d...@redhat.com> wrote: >> >> Hi all, >> >> I was annoyed for some time now by the fact that when I used some >> github-CI-generated RPMs, with a git hash in their names, I could >> never find this git hash anywhere - not in my local git repo, nor in >> github. Why is it so? Because, if I got it right, the default for >> 'actions/checkout@v2' is to merge the PR HEAD with the branch HEAD. >> See e.g. [1]: >> >> HEAD is now at 7bbb40c9a Merge >> 026bb9c672bf46786dd6d16f4cbe0ecfa84c531d into >> 35e217936b5571e9657946b47333a563373047bb >> >> Meaning: my patch was 026bb9c, master was 35e2179, and the generated >> RPMs will have 7bbb40c9a, not to be found anywhere else. If you check >> the main PR page [3], you can find there '026bb9c', but not >> '7bbb40c9a'. >> >> (Even 026bb9c might require some effort, e.g. "didib force-pushed the >> add-hook-log-console branch 2 times, most recently from c90e658 to >> 66ebc88 yesterday". I guess this is the result of github discouraging >> force-pushes, in direct opposite of gerrit, which had a notion of >> different patchsets for a single change. I already ranted about this >> in the past, but that's not the subject of the current message). >> >> This is not just an annoyance, it's a real difference in semantics. In >> gerrit/jenkins days, IIRC most/all projects I worked on, ran CI >> testing/building on the pushed HEAD, and didn't touch it. Rebase, if >> at all, happened either explicitly, or at merge time. > > I don't think that the action *rebases* the pr, it uses a merge commit > but this adds newer commits on master on top of the pr, which may > conflict or change the semantics of the pr. > >> actions/checkout's default, to auto-merge, is probably meant to be >> more "careful" - to test what would happen if the code is merged. I >> agree this makes sense. But I personally think it's almost always ok >> to test on the pushed HEAD and not rebase/merge _implicitely_. >> >> What do you think? > > I agree, this is unexpected and unwanted behavior in particular for > projects that disable merge commits (e.g. vdsm).
merge commits are disabled for all oVirt projects as per https://www.ovirt.org/develop/developer-guide/migrating_to_github.html > >> It should be easy to change, using [2]: >> >> - uses: actions/checkout@v2 >> with: >> ref: ${{ github.event.pull_request.head.sha }} we can really just create a trivial wrapper and replace globally with e.g. - uses: ovirt/checkout > > +1 > > Nir > _______________________________________________ > Devel mailing list -- devel@ovirt.org > To unsubscribe send an email to devel-le...@ovirt.org > Privacy Statement: https://www.ovirt.org/privacy-policy.html > oVirt Code of Conduct: > https://www.ovirt.org/community/about/community-guidelines/ > List Archives: > https://lists.ovirt.org/archives/list/devel@ovirt.org/message/WZ3W6BII34CTQXXLBYJB6W6ECCWEGM4J/ _______________________________________________ Devel mailing list -- devel@ovirt.org To unsubscribe send an email to devel-le...@ovirt.org Privacy Statement: https://www.ovirt.org/privacy-policy.html oVirt Code of Conduct: https://www.ovirt.org/community/about/community-guidelines/ List Archives: https://lists.ovirt.org/archives/list/devel@ovirt.org/message/HLUUV2YMDGN4ZSSLU75ME4K6KUIITFO4/
