Re: [PATCH 1/2] strbuf: add tests

2016-05-31 Thread Simon Rabourg
Hi Michael, Michael Haggerty writes: > Hi, > > Cool that you are working on this! See my comments below. > > On 05/30/2016 12:36 PM, William Duclot wrote: > > Test the strbuf API. Being used throughout all Git the API could be > > considered tested, but adding specific tests makes it easier to

Re: [PATCH 1/2] strbuf: add tests

2016-05-30 Thread Michael Haggerty
Hi, Cool that you are working on this! See my comments below. On 05/30/2016 12:36 PM, William Duclot wrote: > Test the strbuf API. Being used throughout all Git the API could be > considered tested, but adding specific tests makes it easier to improve > and extend the API. > --- > Makefile

Re: [PATCH 1/2] strbuf: add tests

2016-05-30 Thread Simon Rabourg
Hi Johannes, I'm William's teammate on this feature. Johannes Schindelin writes: > Hi William, > > On Mon, 30 May 2016, William Duclot wrote: > > > Test the strbuf API. Being used throughout all Git the API could be > > considered tested, but adding specific tests makes it easier to improve

Re: [PATCH 1/2] strbuf: add tests

2016-05-30 Thread Matthieu Moy
I agree with Johannes' remarks. I'd add two style nits: William Duclot writes: > --- /dev/null > +++ b/t/helper/test-strbuf.c > @@ -0,0 +1,69 @@ > +#include "git-compat-util.h" > +#include "strbuf.h" > + > +/* > + * Check behavior on usual use cases > + */ > +int test_usual(struct strbuf *sb) >

Re: [PATCH 1/2] strbuf: add tests

2016-05-30 Thread Johannes Schindelin
Hi William, On Mon, 30 May 2016, William Duclot wrote: > Test the strbuf API. Being used throughout all Git the API could be > considered tested, but adding specific tests makes it easier to improve > and extend the API. > --- The commit message makes sense. Please add your sign-off. > Makefil

[PATCH 1/2] strbuf: add tests

2016-05-30 Thread William Duclot
Test the strbuf API. Being used throughout all Git the API could be considered tested, but adding specific tests makes it easier to improve and extend the API. --- Makefile | 1 + t/helper/test-strbuf.c | 69 ++ t/t0082-strbuf.sh