Skip to content

Batch send refspec take 2#2809

Merged
technoweenie merged 5 commits intomasterfrom
batch-send-refspec-2
Jan 8, 2018
Merged

Batch send refspec take 2#2809
technoweenie merged 5 commits intomasterfrom
batch-send-refspec-2

Conversation

@technoweenie
Copy link
Contributor

This is a follow-up to #2806, finally teaching the tq.TransferQueue to pass on the refspec to the server.

Copy link
Contributor

@ttaylorr ttaylorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look great to me! I have a few minor comments, but this 👀 ready to go from my perspective.

verifyLocksForUpdates(ctx.lockVerifier, updates)
for _, update := range updates {
q := ctx.NewQueue() // initialized here to prevent looped defer
q := ctx.NewQueue( // initialized here to prevent looped defer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: should this comment be moved to the line above? I think if we ever add arguments to this method, it will help keep the lines short.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I suppose. The comment could be moved when the first comment is added.


refute_server_object "$reponame" 98ea6e4f216f2fb4b69fff9b3a44842c38686ca685f3f55dc48c5d3fb1107be4 "refs/heads/tracked"

# for some reason, using 'tee' and $PIPESTATUS does not work here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that `${PIPESTATUS[1]} would work, since there are two commands being piped here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That must be copy pasta, I don't recall any issues. At any rate, it's just a test so it doesn't matter ;)


# for some reason, using 'tee' and $PIPESTATUS does not work here
set +e
echo "refs/heads/master master refs/heads/master 0000000000000000000000000000000000000000" |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

@technoweenie technoweenie merged commit 96cfc0e into master Jan 8, 2018
@technoweenie technoweenie deleted the batch-send-refspec-2 branch January 8, 2018 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants