Re: cabinet: Revert "cabinet: Fix for FDICopy with an empty cabinet file."

2008-04-26 Thread Reece Dunn
2008/4/26 James Hawkins <[EMAIL PROTECTED]>:
>
> On Sat, Apr 26, 2008 at 1:03 AM, Reece Dunn <[EMAIL PROTECTED]> wrote:
>  > 2008/4/26 James Hawkins <[EMAIL PROTECTED]>:
>  >
>  > > Hi,
>  >  >
>  >  >  The test was incorrect and failed on all platforms.  Vitaly, please
>  >  >  ask someone to test your patches on a real windows system in the
>  >  >  future if you don't have access to one.
>  >
>  >  Wouldn't it be better to actually fix the test case than revert it all 
> together?
>  >
>
>  Have you actually read the patch or the changelog for that matter?

I misread the "Revert ..." part to mean a full revert of the original patch.

Sorry for the noise.
- Reece




Re: cabinet: Revert "cabinet: Fix for FDICopy with an empty cabinet file."

2008-04-26 Thread Reece Dunn
2008/4/26 James Hawkins <[EMAIL PROTECTED]>:
> Hi,
>
>  The test was incorrect and failed on all platforms.  Vitaly, please
>  ask someone to test your patches on a real windows system in the
>  future if you don't have access to one.

Wouldn't it be better to actually fix the test case than revert it all together?

Just simply change line 630:

-ok(ret, "Expected FDICopy to succeed\n");
+ok(!ret, "Expected FDICopy to fail\n");

and remove the todo_wine. Adding a GetLastError check would be good as well.

- Reece




Re: cabinet: Revert "cabinet: Fix for FDICopy with an empty cabinet file."

2008-04-25 Thread James Hawkins
On Sat, Apr 26, 2008 at 1:03 AM, Reece Dunn <[EMAIL PROTECTED]> wrote:
> 2008/4/26 James Hawkins <[EMAIL PROTECTED]>:
>
> > Hi,
>  >
>  >  The test was incorrect and failed on all platforms.  Vitaly, please
>  >  ask someone to test your patches on a real windows system in the
>  >  future if you don't have access to one.
>
>  Wouldn't it be better to actually fix the test case than revert it all 
> together?
>
>  Just simply change line 630:
>
>  -ok(ret, "Expected FDICopy to succeed\n");
>  +ok(!ret, "Expected FDICopy to fail\n");
>
>  and remove the todo_wine. Adding a GetLastError check would be good as well.
>

Have you actually read the patch or the changelog for that matter?

-- 
James Hawkins