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 changeI 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