Re: crypt32: Added Quoting-Tests

[Juan Lang]

Hi Christian,

-static BYTE bin1[] = { 0x55, 0x53 };
-static BYTE bin2[] = { 0x4d, 0x69, 0x6e, 0x6e, 0x65, 0x73, 0x6f, 0x74,
+static BYTE bin1[]  = { 0x55, 0x53 };
+static BYTE bin2[]  = { 0x4d, 0x69, 0x6e, 0x6e, 0x65, 0x73, 0x6f, 0x74,
  0x61 };

Please don't introduce whitespace-only changes, it's harder to see
what your patch is doing.

+ { 0, CERT_RDN_PRINTABLE_STRING,
+   { sizeof(bin9), bin9 }, abc\def },

Since these changes pass on unpatched wine, they don't demonstrate the
necessity for your patch.  We need tests that show what the existing
problem is.
--Juan

Re: crypt32: Added Quoting-Tests

[Christian Inci]

Am 2011-01-11 16:18, schrieb Juan Lang:
 Please don't introduce whitespace-only changes, it's harder to see
 what your patch is doing.
Sorry, that wasn't intended.
 Since these changes pass on unpatched wine, they don't demonstrate the
 necessity for your patch.  We need tests that show what the existing
 problem is.
I don't know, what you've doing, but for me it show errors. (on
unpatched crypt32/str.c (and patched crypt32/tests/str.c ;) ))

--Christian

Re: crypt32: Added Quoting-Tests

[Juan Lang]

 I don't know, what you've doing, but for me it show errors. (on
 unpatched crypt32/str.c (and patched crypt32/tests/str.c ;) ))

Ah, right.  Sorry, I wasn't thinking clearly.  In that case, you need
to number the patches as a series to show the dependency between them.
--Juan

Re: crypt32: Added Quoting-Tests

[Christian Inci]

Am 2011-01-11 16:58, schrieb Juan Lang:
 Ah, right.  Sorry, I wasn't thinking clearly.  In that case, you need
 to number the patches as a series to show the dependency between them.
 --Juan
First the normal then the test, or reverse?
--Christian

Re: crypt32: Added Quoting-Tests

[Juan Lang]

 First the normal then the test, or reverse?

Tests must pass with every commit.  In general, I prefer to see tests
first, with failing tests marked todo_wine.  Then the second patch,
with implementation, removes the todo_wines.  In this case adding
todo_wine might be more trouble than it's worth, so I'd do them in the
reverse order.  I'll comment on your try 2 again shortly, though.
--Juan

Re: crypt32: Added Quoting-Tests

[Christian Inci]

Am 2011-01-11 17:03, schrieb Juan Lang:
 Tests must pass with every commit.  In general, I prefer to see tests
 first, with failing tests marked todo_wine.  Then the second patch,
 with implementation, removes the todo_wines.  In this case adding
 todo_wine might be more trouble than it's worth, so I'd do them in the
 reverse order.  I'll comment on your try 2 again shortly, though.
Sorry for the added blankline.
I'll make both to a same-named try3 shortly.