I wish it were that simple...
Here's the current situation: 1) some months ago I already added tests that demonstrate the command chain problems (which are most of the @todo_wine@ that i'm removing in the commit because they were fixed) 2) IF and FOR actually contain command chains: "IF [«options»] «test» «chain» [ELSE «chain»]" "FOR [«options»] «var» IN («stuff») DO «chain»" 3) the current tests are heavily dependent on IF, FOR and CALL 4) currently IF, ELSE and FOR are parsed in a hackish way (causing the first command to be part of the "if ", "else " or "do " command unless it starts with parens), and because of that particular parsing problem the IF and FOR commands don't follow the normal execution path... for some reason parsing was never fixed and instead we get WCMD_part_execute, which is only needed because of the hackish parsing 5) CALL is also handled in a hackish way that just screams "spaghetti code" (in particular the called/retryCall parameter and all it's hard to grasp side effects) 6) pipes and redirects are also handled in a hackish way that only works for simple cases 7) labels are pretty simple, I just have to make sure they aren't executed
In short: - if I try to fix IF or FOR first then i'm forced to add chains to avoid getting lots of errors - if I try to add chains first then i'm forced to fix IF and FOR to avoid getting lots of errors - if I change FOR then I have to handle the difficult 'CALL FOR' test, which relies on a spagetti of hackish code
So I ask again, considering that I don't see how it can be split while keeping the tests working, how should I proceed? Is it ok if I split it into logical bits and only have the tests working at the end? Maybe splitting the final test file modifications is enough?
Flávio J. Saraiva
2017-03-23 17:14 GMT+00:00 Frédéric Delanoy frederic.delanoy@gmail.com:
On Wed, Mar 22, 2017 at 11:34 AM, Flávio J. Saraiva flaviojs2005@gmail.com wrote:
Hi,
Some time ago I was told that every patch in a patch set all patches should produce a state that passes all the tests. Given how much the current code is interconnected, i don't think i can split this while keeping the tests functional.
How should I proceed in this case, can I split the code into logical bits knowing that the intermediate states will not pass the tests?
Flávio J. Saraiva
You could start by submitting test-only patches first, demonstrating the issues. Then, split the code. For instance, your patch says "Adjust CMD_LIST to a tree-like structure and implement command chainsAdjust CMD_LIST to a tree-like structure and implement command chains", maybe the structure change could be a single patch, ensuring tests still pass? Then add one or more patches fixing the tests.
Just my 2 ¢
-- Frédéric Delanoy
2017-03-21 21:06 GMT+00:00 Marvin testbot@winehq.org:
Thank you for your contribution to Wine!
This is an automated notification to let you know that your patch has been reviewed and its status set to "Needs splitting".
This means that the patch needs to be split, either because it contains unrelated changes that should be their own separate patch, or because it's too large for review. You need to find a way to resend the changes in smaller chunks.
If you do not understand the reason for this status, disagree with our assessment, or are simply not sure how to proceed next, please ask for clarification by replying to this email.