http://bugs.winehq.org/show_bug.cgi?id=17195
--- Comment #127 from Adam Martinson adam.r.martinson@gmail.com 2013-01-03 21:04:20 CST --- (In reply to comment #125)
(In reply to comment #123)
You mean split patch 5 more?
Yes. If some app breaks after your commits, you'll get a regression test pointing to one of them. The smaller that patch is, the better for you. Apps tend to break on good commits too, so even if you're sure everything is OK, splitting is always a good idea.
I know that splitting is good xD But that's the one where you stop getting a pipe_instance handle and start getting a pipe_end handle for the server end. Those functions *must* change at the same time or stuff breaks. Is there a way to get around that that I'm not seeing?
(In reply to comment #124)
well, alexandre already asked for tests to be carried out, and you've already explained that tests cannot be carried out unless the *entire* patch set is applied and accepted, because it's so fundamental.
They don't all have to be committed at once. The 1st one can stand on its own. 2-6 are the 1st chunk, redesigning the backend. 7-9 are the 2nd chunk, implementing {Get|Set}NamedPipeHandleState() and the FILE_PIPE_INFORMATION stuff. 10 is the last chunk, I just don't know how to split that one without breaking stuff.
He didn't have any problem with the tests that I recall, he just wanted the major patch from the 1st chunk split AFAIK, so I don't think a switch is needed.
it's not obvious just from the patch what's going on. is it *only* renaming? if so, what gets renamed to what? if you only renamed one at a time, the number of lines being changed would be smaller, so comparisons would be easy.
The patches that say (no-op) are only renames. part 1: ps_connected_server => ps_connected ps_wait_disconnect => ps_disconnected_client client => end part 2: instances => numinstances server => instance servers => instances event => event_empty
the sheer number of things being renamed here actually removes *entire* functions and replaces them with ones that are... identical? are they? i don't know, and i can't tell.
If you use a diff tool that shows changes inside lines it's easier. I'd rather not create a whole bunch of no-op patches unless Alexandre thinks it's necessary.
this would at least allow those to be done as entirely independent separate patches that would clearly have nothing to do with the actual functionality.
That's what the (no-op) in the description is for ;-)