Total overhaul of the fetch loop for v4 #691
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: thockin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
This was referenced Feb 26, 2023
Closed
|
I am so sorry for this one :) |
k8s-ci-robot
added
the
needs-rebase
The previous (v3) sync loop betrays my lack of understanding about git. It tried to codify my archaic mental model (e.g. --branch and --rev being disting things) and was ultimately a patchwork of corner-cases evolved over a few years. This commit is less of a "diff" and more of a "rewrite". The new logic is simpler and more efficient. It does not `git clone` ever. It does not differentiate the first sync from subsequent syncs. It uses `git fetch` to get the exact SHA and then makes a worktree from that. The new `--ref` flag replaces both `--rev` and `--branch`, though it will use those if specified. In fact, almost all of the e2e tests passed without change - using --ref and --branch! I will follow this commit up with more cleanups and e2es.
thockin
force-pushed
the
overhaul_fetch_loop_for_v4
branch
from
32cc933
to
35323a2
Compare
k8s-ci-robot
removed
the
needs-rebase
|
I self-merged some of the smaller ones before this, but stopped here for now. I'd love to have extra eyes on it, but if everyone is too busy, I don't mind pushing ahead to shoot for a v4 RC. |
|
self-merge to make progress |
|
Damn thought you self merged all. Will review after the fact once I have some capacity. So don't mind you merging your progress. |
|
No sweat, this was a LOT of load to dump, and I have more queued up! :)
…On Mon, Apr 24, 2023, 1:35 AM Michael Grosser ***@***.***> wrote:
Damn thought you self merged all. Will review after the fact once I have
some capacity. So don't mind you merging your progress.
—
Reply to this email directly, view it on GitHub
<#691 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABKWAVBCVGMCLEZGVXNR6CLXCY3O3ANCNFSM6AAAAAAVIDM3LU>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This one is a doozy but can't easily be broken up further. I have a dozen more on top of this, building to a v4 release candidate, I hope.
The previous (v3) sync loop betrays my lack of understanding about git. It tried to codify my archaic mental model (e.g. --branch and --rev being disting things) and was ultimately a patchwork of corner-cases evolved over a few years.
This commit is less of a "diff" and more of a "rewrite".
The new logic is simpler and more efficient. It does not
git cloneever. It does not differentiate the first sync from subsequent syncs. It usesgit fetchto get the exact SHA and then makes a worktree from that.The new
--refflag replaces both--revand--branch, though it will use those if specified. In fact, almost all of the e2e tests passed without change - using --ref and --branch!I will follow this commit up with more cleanups and e2es.