> On 21. 7. 2022, at 9:09, Yedidyah Bar David <[email protected]> wrote: > > On Fri, Jul 8, 2022 at 11:30 AM Martin Perina <[email protected] > <mailto:[email protected]>> wrote: > > > On Fri, Jul 8, 2022 at 10:27 AM Michal Skrivanek <[email protected] > <mailto:[email protected]>> wrote: > > > > On 7. 7. 2022, at 19:28, Nir Soffer <[email protected] > > <mailto:[email protected]>> wrote: > > > > On Wed, Jun 15, 2022 at 12:26 PM Yedidyah Bar David <[email protected] > > <mailto:[email protected]>> 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 > <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 > > As this needs to be included in each project separately, then I'd say let's > minimize available options to ensure maximum consistency across all oVirt > projects > > 1. I don't know how, and would have to learn quite a bit of github, to do > this. That's the main reason I neglected this in my TODO folder and didn't > reply yet. Perhaps someone already did something similar and would like to > take over?
Take a look at https://github.com/oVirt/upload-rpms-action minus tests (I hope Janos is not looking)...that makes it a new repo, and license, readme, and yaml file with that snippet. that's it. > > 2. I already pushed (2 weeks ago) and merged (yesterday) to otopi, [1], which > simply does the above. > > 3. Scott now pushed [2], to the engine, doing the same, and I agree with him. > So am going to merge it soon, unless there are objections. If eventually > someone creates an oVirt action for this, we can always update to use it. > > Best regards, > > [1] https://github.com/oVirt/otopi/pull/25 > <https://github.com/oVirt/otopi/pull/25> > > [2] https://github.com/oVirt/ovirt-engine/pull/543 > <https://github.com/oVirt/ovirt-engine/pull/543> > > > > > > +1 > > > > Nir > > _______________________________________________ > > Devel mailing list -- [email protected] <mailto:[email protected]> > > To unsubscribe send an email to [email protected] > > <mailto:[email protected]> > > Privacy Statement: https://www.ovirt.org/privacy-policy.html > > <https://www.ovirt.org/privacy-policy.html> > > oVirt Code of Conduct: > > https://www.ovirt.org/community/about/community-guidelines/ > > <https://www.ovirt.org/community/about/community-guidelines/> > > List Archives: > > https://lists.ovirt.org/archives/list/[email protected]/message/WZ3W6BII34CTQXXLBYJB6W6ECCWEGM4J/ > > > > <https://lists.ovirt.org/archives/list/[email protected]/message/WZ3W6BII34CTQXXLBYJB6W6ECCWEGM4J/> > _______________________________________________ > Devel mailing list -- [email protected] <mailto:[email protected]> > To unsubscribe send an email to [email protected] > <mailto:[email protected]> > Privacy Statement: https://www.ovirt.org/privacy-policy.html > <https://www.ovirt.org/privacy-policy.html> > oVirt Code of Conduct: > https://www.ovirt.org/community/about/community-guidelines/ > <https://www.ovirt.org/community/about/community-guidelines/> > List Archives: > https://lists.ovirt.org/archives/list/[email protected]/message/HLUUV2YMDGN4ZSSLU75ME4K6KUIITFO4/ > > <https://lists.ovirt.org/archives/list/[email protected]/message/HLUUV2YMDGN4ZSSLU75ME4K6KUIITFO4/> > > > -- > Martin Perina > Manager, Software Engineering > Red Hat Czech s.r.o. > > > -- > Didi
_______________________________________________ Devel mailing list -- [email protected] To unsubscribe send an email to [email protected] 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/[email protected]/message/KICLSYKGKB7EXI3LFLBJKMW2LA5P5AIH/
