Re: [PATCH] msi: Specify in FIXME that functions are semi-stub as for MsiMessageBoxEx. (resend)

[Alexandre Julliard]

Christian Costa  writes:
> {
> - FIXME("%p %s %s %u %08x %08x\n", hWnd, debugstr_w(lpText), debugstr_w(lpCaption),
> -
>      uType, wLanguageId, f);
> - return MessageBoxExW(hWnd,lpText,lpCaption,uType,wLanguageId);
> - FIXME("(%p, %s, %s, %u, 0x%08x, 0x%08x): semi-stub\n", hWnd, debugstr_w(lpText),
> -
>        debugstr_w(lpCaption), uType, wLanguageId, f);
> - return MessageBoxExW(hWnd,lpText,lpCaption,uType,wLanguageId);

That sort of thing is not useful. If you are changing the function for
other reasons you can update the trace, but otherwise it's just adding
noise in the change history.

[PATCH] msi: Specify in FIXME that functions are semi-stub as for MsiMessageBoxEx. (resend)

[Christian Costa]

Le 08/05/2013 12:43, Alexandre Julliard a écrit :
> Christian Costa  writes:
> > {
> > - FIXME("%p %s %s %u %08x %08x\n", hWnd, debugstr_w(lpText), debugstr_w(lpCaption),
> > -
> >      uType, wLanguageId, f);
> > - return MessageBoxExW(hWnd,lpText,lpCaption,uType,wLanguageId);
> > - FIXME("(%p, %s, %s, %u, 0x%08x, 0x%08x): semi-stub\n", hWnd, debugstr_w(lpText),
> > -
> >        debugstr_w(lpCaption), uType, wLanguageId, f);
> > - return MessageBoxExW(hWnd,lpText,lpCaption,uType,wLanguageId);
>
> That sort of thing is not useful. If you are changing the function for
> other reasons you can update the trace, but otherwise it's just adding
> noise in the change history.

It's good to have clearer fixme messages. They are enabled by default so 
we don't have to bother again about their meaning.
Sometimes it's just leftover fixme. I have already fixed some of them 
which had the same form by turning them into traces.
With this patch they can't be interpreted as other leftover fixme.

[PATCH] msi: Specify in FIXME that functions are semi-stub as for MsiMessageBoxEx. (resend)

[Alexandre Julliard]

Christian Costa  writes:
> Le 08/05/2013 12:43, Alexandre Julliard a écrit :
> > Christian Costa  writes:
> > > {
> > > - FIXME("%p %s %s %u %08x %08x\n", hWnd, debugstr_w(lpText), debugstr_w(lpCaption),
> > > -
> > >      uType, wLanguageId, f);
> > > - return MessageBoxExW(hWnd,lpText,lpCaption,uType,wLanguageId);
> > > - FIXME("(%p, %s, %s, %u, 0x%08x, 0x%08x): semi-stub\n", hWnd, debugstr_w(lpText),
> > > -
> > >        debugstr_w(lpCaption), uType, wLanguageId, f);
> > > - return MessageBoxExW(hWnd,lpText,lpCaption,uType,wLanguageId);
> >
> > That sort of thing is not useful. If you are changing the function for
> > other reasons you can update the trace, but otherwise it's just adding
> > noise in the change history.
>
> It's good to have clearer fixme messages. They are enabled by default
> so we don't have to bother again about their meaning.
> Sometimes it's just leftover fixme. I have already fixed some of them
> which had the same form by turning them into traces.
> With this patch they can't be interpreted as other leftover fixme.

Then instead of leftover fixmes without text, we'll have leftover fixmes
with "semi-stub" in them. I don't see the improvement.

The usefulness is in knowing that this function was called, so that if
there are problems in that area it points out a possible culprit. Making
it say "stub" or "semi-stub" or "sort-of-working-but-maybe-not-quite"
doesn't provide any useful additional information in the trace.