8:31 a.m.
On Thu, Jun 16, 2022 at 1:27 PM Yedidyah Bar David <didi(a)redhat.com> wrote:
On Wed, Jun 15, 2022 at 8:31 PM Michal Skrivanek <mskrivan(a)redhat.com> wrote:
>
>
>
> > On 15. 6. 2022, at 11:25, Yedidyah Bar David <didi(a)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?
>
> huh, I wondered about that same thing today....
> Thank you for explaining why I couldn't find that hash anywhere
>
> > 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).
>
> We should create ovirt-github-rants(a)ovirt.org, I'd certainly contribute:-)
It's amazing how horrible _and_ popular github is.
>
> >
> > 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.
> >
> > 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?
> >
> > It should be easy to change, using [2]:
> >
> > - uses: actions/checkout@v2
> > with:
> > ref: ${{ github.event.pull_request.head.sha }}
> >
> > No need to reach a complete consensus - can be decided upon
> > per-project/repo.
>
> github is always quite horrible to maintain some consistency across projects...yeah,
I'd really like to have the same approach for every single project, it simplifies the
maintenace....we do have a lot of projects and many are not very active and they easily
fall behind. After all we have 160 projects in oVirt org but only ~30 are active....or
rather 30 are in use for oVirt compose and ~10 are active.
>
> +1 on using it everywhere
> we have our own action for rpms and buildcontainer for unified build environment
(with a shameful exception of vdsm!)....it's probably overkill for checkout to use
oVirt's action
>
> > But if you disagree, I'd like to understand why.
If someone _wants_ the current behavior (merge to HEAD) but is just
annoyed by the commit hash, an alternative is to use the PR hash in
the name. Now tried this POC, seems to work:
https://github.com/oVirt/otopi/pull/23
Also this one. Much simpler, but perhaps we still want the refactoring of ^^.
https://github.com/oVirt/otopi/pull/24
Best regards,
> > Thanks.
> >
> > Best regards,
> >
> > [1] https://github.com/oVirt/vdsm/runs/6881311961?check_suite_focus=true
> >
> > [2]
https://github.com/marketplace/actions/checkout?version=v2.4.2#checkout-p...
> >
> > [3] https://github.com/oVirt/vdsm/pull/249
> > --
> > Didi
> > _______________________________________________
> > Devel mailing list -- devel(a)ovirt.org
> > To unsubscribe send an email to devel-leave(a)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/7SEFKOASOAT...
>
--
Didi
--
Didi