Open immortaleeb opened 2 years ago
Thank you for this pull request.
I took the time to download it and run it. I did not look at the code yet. There is something unexpected about the result of this change.
I ran the following scenario:
This simulation's WIP peaks at more than 100.
I'm wondering if this is a logical consequence of the simulation or if there is a reasoning error somewhere.
In the previous version, a worker would always look at the columns from right to lef to pick up a next task. I have a feeling the new addition could have reversed that.
Hey Michel, thanks for the feedback.
I actually think the behaviour is a consequence of the simulation you've set up:
As a small test, I modified the visualisation of each item so it also displays which workers worked on that item at a given time. E.g. 100 (1, 2, 3)
means that item 100 was worked on by worker 1, then worker 2, then worker 3.
I then ran the simulation twice with the parameters you provided. Once with unique stories enabled and another time with it disabled.
Here's a snapshot of when unique stories is disabled:
As you can see, workers do pick up work from right to left, but - because they're all fullstack
workers - it's actually the same worker who completes the work that also picks up the same ticket to be processed on next. I actually didn't expect this, so I reran the same simulation on the master
branch to check if I made any mistakes but got the same results.
Next up is a snapshot of the same simulation with unique stories enabled:
What you can see here is that we quickly get into a stable state where worker 2 is doing all the ux work while workers 1 and 3 are doing the dev and qa work. The reason worker 2 doesn't pick up any dev
or qa
work is because it already worked on those tickets while doing ux
. Since dev
work takes 3 times more work than ux
, it makes sense then that a queue would build up before dev
.
Perhaps we should expand the feature to where we can say things like "a worker who did ux is also allowed to do the dev work, but qa always needs to be done by a different worker"? It's definitely an option, but it would require more development work and some rethinking about how best to tackle this.
Regardless of the changes in this PR, should we consider it a bug that a worker picks up their own work without giving another worker the chance to pick it up?
Thank you for your investigation. Your explanation makes sense. Although I don't like the outcome of the simulation, I understand it's correct.
However, I ran a few scenarios where this strategy blocks certain items. I was experimenting with the scenario from #21:
I then tried to improve it by making the last worker fullstack:
In this scenario, some items don't make it to the done column. I suspect those are the ones where ux has been done by the fullstack worker, and dev has been done by the dev+pr worker, leaving no-one to do the actual pr.
Although this is 100% correct, this is not easily predictable when tuning scenarios. How would you feel if we loosened the constraint to: a worker is not allowed to pick up the next part of a task they just finished?
e.g.:
This way, the blockage is less likely, more predictable, and even acceptable in real life.
Regarding your last question:
should we consider it a bug that a worker picks up their own work without giving another worker the chance to pick it up?
I don't consider this a bug. It can be a team agreement to work like this, even if it's not a very good one.
This adds a checkbox to the UI called "Work on unique stories". When enabled, each worker will only be able to work on items they haven't worked on before. I got the idea from https://github.com/michelgrootjans/explaining-flow/issues/21#issuecomment-911333200
This was implemented by extracting a strategy for each
Worker
. The strategy decides which items can be worked on, at what speed and what the name of each worker is. Currently two strategies are provided:SimpleSkillStrategy
implements the default/old behaviour where a worker can only work on items in a column when it has the correct skills for that columnDontWorkOnSameItemStrategy
is implemented as a decorator around other strategies which forces the worker to only work on a given item once (e.g. if a worker worked on something in dev, then the same worker can't later do the pr as well).Note that I had some trouble naming things for this feature. I'm not convinced that
don't work on the same item
andwork on unique items
are good names, but I've yet to find a better alternative.Hope this helps and if you have any suggestions or see any improvements then let me know.