Thread (126 messages) 126 messages, 9 authors, 2024-12-05

Re: [PATCH v12 7/8] fetch: set remote/HEAD if it does not exist

From: Bence Ferdinandy <hidden>
Date: 2024-10-23 17:07:46

On Wed Oct 23, 2024 at 18:50, Kristoffer Haugsbakk [off-list ref] wrote:
On Wed, Oct 23, 2024, at 17:36, Bence Ferdinandy wrote:
quoted
If the user has remote/HEAD set already and it looks like it has changed
on the server, then print a message, otherwise set it if we can.
Silently pass if the user already has the same remote/HEAD set as
reported by the server or if we encounter any errors along the way.

Signed-off-by: Bence Ferdinandy <redacted>
---

Notes:
    v3: - does not rely on remote set-head anymore so it only authenticates
        once
        - uses the new REF_CREATE_ONLY to atomically check if the ref exists
          and only write it if it doesn't
        - in all other cases the maximum it does is print a warning

    v4: - instead of the discarded REF_CREATE_ONLY, it uses the existing,
          but updated transaction api to request a silent create only
        - it now uses the atomic before_target to determine reporting
        - refactored for legibility

    v5: - instead of printing a not too useful message, it now fails
          silently, this in line with the objective to only set up
          remote/HEAD automatically if the right thing is trivial, for
          everything else there is remote set-head
        - fixed all failing tests
        - added two new tests, one for checking if remote/HEAD is set to the
          correct one, and one to test that we do not override remote/HEAD
          if it has changed on the server from what we have locally

    v6: - fixed style issues and unintended extra empty line
        - updated function call with bool to int from previous patch's
          change
        - removed calls to error(...) inherited from builtin/remote.c so we
          actually fail silently
        - set the test for remote set-head --auto to the correct value here,
          which was previously erronously set in the remote set-head patch

    v7: - no change

    v8: - changed logmsg in call to refs_update_symref from "remote
          set-head" to "fetch"

    v9: - follow through with refs_update_symref_extended
        - fix test errors uncovered by the new patch

    v10: no change

    v11: fixed some memory leaks

    v12: no change
I think it would be better to reverse-order these patch changelog
comments so that the newest is on top/first.  (for next time)

Thanks for the careful versioning here.
Yeah, this works fine when you only go up to v3 and it all fits on a screen :D
This is definitely the longest patch series I've made thus far, both in number
of commits and versions. I also noticed this today when I realized that I did the
cover letter in reverse and how much better readable that was than some of the
patches ...

One thing that would not be easy is that with so many patches, how I start out
the notes is

	git rev-list HEAD~8..HEAD | xargs -i git notes append {} -m "v12: no change"

and there's no "git notes prepend". That could be another patch for another
time :) It also bothers me, that the branch description that can be used to
save cover letters is not a note, just a local configuration, but that's
a third issue.

Anyhow, thanks for calling it out, if we do come to a v13 I might just spend
some time on reversing (or be lazy and just continue at the top ...).

Best,
Bence

-- 
bence.ferdinandy.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help