On Sun, 17 Mar 2013 15:14:32 -0700 Junio C Hamano <gitster@xxxxxxxxx> wrote: > Rob Hoelz <rob@xxxxxxxx> writes: > > > Hi everyone! I found a bug in Git today and wrote up a fix; I did > > my best to conform to the rules layed out in > > Documentation/SubmittingPatches, but please let me know if I need > > to change anything to get my work merged. =) I have CC'ed Josh > > Triplet, as he was the last one to touch the line I modified. I > > hope my commit messages explain the problem I encountered well > > enough; if not, I can always go back and amend them. > > > > Patches follow. > > > > -Rob > > > Please read Documentation/SubmittingPatches and follow it. The > above is most likely to be the cover letter of a two-patch series > (meaning you will be sending three pieces of e-mail messages), or > perhaps out of band comment below the three-dash line of a single > patch (you will send only one piece of e-mail message). > > See recent patches on the list from list regulars for good examples, > e.g. > > http://thread.gmane.org/gmane.comp.version-control.git/218350 > http://thread.gmane.org/gmane.comp.version-control.git/218177/focus=218361 > > > From 5007b11e86c0835807632cb41e6cfa75ce9a1aa1 Mon Sep 17 00:00:00 > > 2001 From: Rob Hoelz <rob@xxxxxxxx> > > Date: Sun, 17 Mar 2013 21:49:20 +0100 > > Subject: [PATCH 1/2] Add test for pushInsteadOf + pushurl > > > > git push currently doesn't consider pushInsteadOf when > > using pushurl; this test tests that. > > > > Signed-off-by: Rob Hoelz <rob@xxxxxxxx> > > --- > > t/t5500-push-pushurl.sh | 37 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > create mode 100644 t/t5500-push-pushurl.sh > > The number 5500 is already taken. Please do not add a duplicate. > > I also wonder if we need to waste a new test number for this; > perhaps adding new tests to 5516 that already tests insteadOf might > be a better fit, but I didn't carefully read it. > > > diff --git a/t/t5500-push-pushurl.sh b/t/t5500-push-pushurl.sh > > new file mode 100644 > > Test scripts are supposed to be executable. > > > +test_expect_success 'test commit and push' ' > > + test_commit one && > > + git push origin master:master > > +' > > + > > +test_expect_success 'check for commits in rw repo' ' > > + cd ../rw/repo && > > + git log --pretty=oneline | grep -q . > > +' > > Are both expected to succeed in patch 1/2 without any code change? > > If you were doing a large code change, it is a good series structure > to have tests first that are marked as "expect_failure" in an early > patch, and then in a later patch that changes the code to fix it, > update the tests that start to pass to "expect_success". > > I personally do not think you need such a two-step approach for > something small like this; instead you can just have a single patch > that adds a set of tests that expect success and code change. > > Thanks. > Thanks for the feeback; another reply with the new patch follows. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html