Re: winmm: MCI system commands are not eligible for auto-open. (try 2)

2010-04-13 Thread Juan Lang
Hi Dmitry, > And another rule is that the patch which changes the behaviour of an API > needs to have an appropriate test case which does not pass before the patch > (i.e. has the todo_wine around it), and passes after the patch (i.e. the patch > removes the corresponding todo_wine). Your patch do

Re: winmm: MCI system commands are not eligible for auto-open. (try 2)

2010-04-12 Thread Dmitry Timoshkov
wrote: > >Since there is no neither todo_wine statements in the tests, nor test > >failures > >under Wine that means that both the tests and the patch are not OK. > > What has Wine to say when talking about the correctness > of tests? Only native dictates what the test result should be. Of co

winmm: MCI system commands are not eligible for auto-open. (try 2)

2010-04-12 Thread Joerg-Cyril.Hoehle
Dmitry Timoshkov wrote: >Since there is no neither todo_wine statements in the tests, nor test failures >under Wine that means that both the tests and the patch are not OK. What has Wine to say when talking about the correctness of tests? Only native dictates what the test result should be. The

Re: winmm: MCI system commands are not eligible for auto-open. (try 2)

2010-04-12 Thread Dmitry Timoshkov
wrote: > >I don't see any corresponding changes in that patch that remove todo_wine > >statements > >in the tests. That suggests that either such tests don't exist, or that > >change actually > >fixes nothing. > A third explanation is possible: > It's an example of one bug shadowing another bug

winmm: MCI system commands are not eligible for auto-open. (try 2)

2010-04-12 Thread Joerg-Cyril.Hoehle
Dmitry Timoshkov wrote: >I don't see any corresponding changes in that patch that remove todo_wine >statements >in the tests. That suggests that either such tests don't exist, or that change >actually >fixes nothing. A third explanation is possible: It's an example of one bug shadowing another b

Re: winmm: MCI system commands are not eligible for auto-open. (try 2)

2010-04-12 Thread Dmitry Timoshkov
[cc:ing wine-devel] wrote: > this is the second time that AJ commits a patch of mine despite > a comment of yours. I think that Alexandre has commited your patch slightly in a hurry, usually he waits at least several hours unless the patch is obviously correct (which is not the case here IMO).

Re: winmm: MCI system commands are not eligible for auto-open. (try 2)

2010-04-12 Thread Alexandre Julliard
writes: > Ok. After being bitten at least once by assignment/comparison > mismatch I promised myself to use that style. I'm myself used > to read code as "if A equals 3" rather than "if 3 is the value > of A" but I'm convinced that's just a matter of getting used > to this style that is less er

Re: winmm: MCI system commands are not eligible for auto-open. (try 2)

2010-04-12 Thread Dmitry Timoshkov
wrote: > >> +FIXME("(%04x) vkey %04X stub\n", dwFlags, lpParms->nVirtKey); > >That change is unwanted. > I can remove it, but why? Is supporting break keys a WONTFIX in Wine? If that's really a problem then the first thing to do is start with some bug reports or test cases. Adding spurious F

winmm: MCI system commands are not eligible for auto-open. (try 2)

2010-04-12 Thread Joerg-Cyril.Hoehle
Dmitry Timoshkov wrote: >> +FIXME("(%04x) vkey %04X stub\n", dwFlags, lpParms->nVirtKey); >That change is unwanted. I can remove it, but why? Is supporting break keys a WONTFIX in Wine? >Do you have a test case which shows that notofication is not sent is the >failure case? It's already in

Re: winmm: MCI system commands are not eligible for auto-open. (try 2)

2010-04-12 Thread Dmitry Timoshkov
wrote: > +FIXME("(%04x) vkey %04X stub\n", dwFlags, lpParms->nVirtKey); That change is unwanted. > -if (dwFlags & MCI_NOTIFY) > - mciDriverNotify((HWND)lpParms->dwCallback, wDevID, > -(dwRet == 0) ? MCI_NOTIFY_SUCCESSFUL : > MCI_NOTIFY_FAILURE); > - > +