Re: [Qemu-devel] [PULL v2 00/44] rework input handling, sdl2 support

[Peter Maydell]

On 27 February 2014 13:19, Gerd Hoffmann <address@hidden> wrote:
>> I hate bouncing this pull request for a second time, but
>> this still contains stuff that's obviously not ready:
>>  * patch which adds a 1000 line new file, then another
>>    patch 2 commits later deletes 100 of those lines
>
> That is just a case of having forgotten to specify '-M' switch for
> git-format-patch.  Hmm, seems there is no config option to flip the
> default according to 'git config --help'.
>
> It's not a new file, it is "git mv ui/input.c ui/input-legacy.c".

No, I mean ui/sdl2.c.

>>  * "build fix" patch -- we shouldn't break the build in one
>>    patch and fix it in another, this breaks bisection
>
> I wanted to keep patch authorship, that's why it isn't squashed in.
> Also it is in the braille support bits which are not compiled unless you
> have brlapi-devel installed.  I expect this being pretty unusual, so
> most people will not see it.

I expect quite a few people will have brlapi-devel installed
because it's useful to have all the build-depends for optional
bits of QEMU around so you can check you haven't accidentally
broken them. I have it installed, for example.

If you need to fix up somebody else's patch because they've
dumped it on the list and run away without fixing review
issues, the right way to do this is:
 * retain their email as the author
 * retain their Signed-off-by line
 * after that, include a brief summary in square brackets
   of the changes you made to it to fix it up, eg
 [Gerd: fixed style issues, removed unnecessary code, fixed
  breaking of baum2 code] (feel free to be more verbose)
 * then add your Signed-off-by line underneath it

>>  * patches which don't come anywhere near passing
>>    checkpatch
>
> Same reason, keep patch authorship.  Dave's SDL2 patch has a bunch of
> codestyle issues, mostly due to using the SDl1 code as base.  They are
> fixed up later in the series, by deleting code not needed any step by
> step, and finally with a codestyle cleanup patch.

Yes. Don't do this. Patches to be committed should be correct
to start with, not initially broken and then fixed up in
later patches.

thanks
-- PMM