re: [PATCH] Fix EndDialog behaviour for OrCad.

[Dan Kegel]

Thanks for sending that patch.

It seems your editor stripped whitespace from the ends of lines,
causing three whitespace-only hunks on the end of your patch.
Please don't do that; Wine deveopers frown on unrelated whitespace changes.

You might have better luck getting your change in if you include a test.

Even then, we're in code freeze, your fix might have to wait until 1.2
is released.

[PATCH] Fix EndDialog behaviour for OrCad.

[Eliot Blennerhassett]

Hi Dan,

thanks for your response.

On 09/07/10 14:22, Dan Kegel wrote:
> Thanks for sending that patch.
>
> It seems your editor stripped whitespace from the ends of lines,

Having one's editor take care of trailing whitespace is fairly standard
practice isn't it?

Can I guess that having trailing whitespace in my new code would also be
frowned upon?
> causing three whitespace-only hunks on the end of your patch.
> Please don't do that; Wine developers frown on unrelated whitespace changes.

I'm not worried about people "frowning" at me.

If you are saying they won't accept a patch because of this, I'd be
tempted to say "for f**ks sake can I be bothered with such pettiness?",
but I suppose "for the greater good" I'd resubmit.

BTW do patches get *rejected* or just *ignored*, and is there a way to
tell the difference?
> You might have better luck getting your change in if you include a test.

I have absolutely no idea how to or what to test, other than to say
OrCad works with this patch, doesn't work without,
and AFAIK the patch doesn't make any existing test fail.
These existing tests include dialogs that call EndDialog from their
dialog proc.

Without help from knowledgeable wine developers I have no chance of
doing this correctly, and so far I have found interactions on the IRC
channel singularly unhelpful.
> Even then, we're in code freeze, your fix might have to wait until 1.2
> is released.

regards

[PATCH] Fix EndDialog behaviour for OrCad.

[Dan Kegel]

On Thu, Jul 8, 2010 at 7:50 PM, Eliot Blennerhassett
 wrote:
> > It seems your editor stripped whitespace from the ends of lines,
>
> Having one's editor take care of trailing whitespace is fairly standard
> practice isn't it?

Not when working on Wine code.
> Can I guess that having trailing whitespace in my new code would also be
> frowned upon?

Yes.
> If you are saying they won't accept a patch because of this, I'd be
> tempted to say "for f**ks sake can I be bothered with such pettiness?",
> but I suppose "for the greater good" I'd resubmit.

The smaller the patches, the easier they are to review.  And anything we
can do to make the maintainer's life easy is a good thing.
> BTW do patches get *rejected* or just *ignored*, and is there a way to
> tell the difference?

See http://wiki.winehq.org/SubmittingPatches for the link to the patch
status page;
patches are either rejected or deferred, but they don't get ignored.
> > You might have better luck getting your change in if you include a test.
>
> I have absolutely no idea how to or what to test

Can you come up with a small C program that fails without the patch?
- Dan

[PATCH] Fix EndDialog behaviour for OrCad.

[Wolfram Sang]

> > If you are saying they won't accept a patch because of this, I'd be
> > tempted to say "for f**ks sake can I be bothered with such pettiness?",
> > but I suppose "for the greater good" I'd resubmit.

It is common behaviour for large scale projects, e.g. the Linux Kernel 
applies this rule, too. If you frequently work with the history (git 
blame|log), you can soon see the advantages of it. So, it is really not 
there to make sending patches harder ;)

Regards,

Wolfram

[PATCH] Fix EndDialog behaviour for OrCad.

[Eliot Blennerhassett]

On 09/07/10 14:22, Dan Kegel wrote:
> Thanks for sending that patch.
>
> It seems your editor stripped whitespace from the ends of lines,
> causing three whitespace-only hunks on the end of your patch.
> Please don't do that; Wine deveopers frown on unrelated whitespace changes.
>
> You might have better luck getting your change in if you include a test.

Now I see that revised patch 63334 is in "Needs Tests" state, I'm not a
windows application developer...

I am not able to write any new test for this.

There are already 52 uses of EndDialog in dlls/user32/tests
If these still pass...?

OrCad works with my changes.
LTSpice and PADS work as well as they did before.

this is as far as I can go with it.

regards

Eliot

[PATCH] Fix EndDialog behaviour for OrCad.

[Dan Kegel]

On Fri, Jul 9, 2010 at 1:04 PM, Eliot Blennerhassett
 wrote:
> Now I see that revised patch 63334 is in "Needs Tests" state, I'm not a
> windows application developer...
>
> I am not able to write any new test for this.
>
> There are already 52 uses of EndDialog in dlls/user32/tests
> If these still pass...?
>
> OrCad works with my changes.
> LTSpice and PADS work as well as they did before.
>
> this is as far as I can go with it.

OK, I've added a link to your patch from
http://bugs.winehq.org/show_bug.cgi?id=3023
and noted that it needs a test added.

Hopefully somebody else who needs OrCad will come along
and write that test.  (If you know any large sites that need this,
let us know.)
- Dan