3 weeks later, no-one other than Michal replied...
Adding a few more people trying to get their attention.
On Thu, Jun 16, 2022 at 3:31 PM Yedidyah Bar David <
didi@redhat.com> wrote:
>
> On Thu, Jun 16, 2022 at 1:27 PM Yedidyah Bar David <
didi@redhat.com> wrote:
> >
> > On Wed, Jun 15, 2022 at 8:31 PM Michal Skrivanek <
mskrivan@redhat.com> wrote:
> > >
> > >
> > >
> > > > On 15. 6. 2022, at 11:25, Yedidyah Bar David <
didi@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@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
Yes, that's my own preference as well. I am going to push patches to a few
projects doing this, perhaps including ones I am not very active in. Feel
free to ignore/disagree and I'll close.
> > >
> > > > 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(Going to close both of the above)
https://lists.ovirt.org/archives/list/devel@ovirt.org/message/JKI57UOHTMQE3GWBAYQHZ4LTZ5PKLLI3/