Adding Pre-Push Callback Support #1061
Conversation
|
Currently return anything except 0 from the hook will cause an exception to be thrown in the managed layer. I'm not sure this is what we want to be seeing. Perhaps it is. Having a hook block a push for any reason might not be considered exceptional. I'm looking for feedback on this. Once we have the answer to the above, I'll complete the negative test case to insure than returning a failing value blocks the upload portion of the push. |
| /// Called once between the negotiation step and the upload. It provides | ||
| /// information about what updates will be performed. | ||
| /// </summary> | ||
| public PrePushHookHandler OnNegotationCompletedBeforePush { get; set; } |
There was a problem hiding this comment.
As a senior member of the American Bad Spellers Society Id like to thank you for recognition of my (in)abilities. 😆
I think it is reasonable for an exception generated in the push handler to block the push. If someone is relying on the hook to approve changes, then I would like for an error to block the push. |
|
@whoisj Thanks for the PR 🎆 ! I think this will be nice addition. I had some feedback and weighed in on your question. |
|
I believe the PR is all fixed up based on the feedback so far. I found a handy |
|
darn github |
whoisj
changed the title
|
I believe everything is hunky-dory /CC @nulltoken @jamill |
| PackBuilderProgressHandler packBuilderCb = (x, y, z) => { packBuilderCalled = true; return true; }; | ||
| PrePushHandler prePushHook = (IEnumerable<PushUpdate> updates) => | ||
| { | ||
| Assert.True(updates.Count() == 1, "Expected 1 update, recieved " + updates.Count()); |
|
@nulltoken spelling errors corrected in the test code 😜 |
|
🆒 Before we merge this in, could you please sprinkle a bit of your squashing magic on this PR? |
Added `PushUpdate` class, updated `GitPushUpdate` struct Completed plumbing from libgit2 => API layer Added both positive and negative tests to the `PushFixture`
Done sir, all squashed up. 😇 |
Adding Pre-Push Callback Support
|
💥 |
added PushUpdate class, updated GitPushUpdate struct
completed plumbing from libgit2 => API layer
added a positive test, failed to get a negative test to work correctly