On 07:18 pm, itamar@itamarst.org wrote:
Ticket #3690 was filed as "serial ports do not call connectionLost on their protocol", but along the way became a ticket for testing *all* of
I don't think this is quite right. The blocker was having *any* tests.
the serial port code. Since the latter is much harder, it got stuck in addressing review comments, and the simpler original problem was not fixed. In this case, the perfect was the enemy of the good.
To prevent this sort of thing, if you end up enlarging the scope of a ticket dramatically, please open a new ticket and divide the work up. Splitting up large amounts of work into smaller units is good even if the scope of the ticket hasn't changed.
Smaller units of work are easier on everyone, developers and reviewers; please look for opportunities to break up tickets into multiple smaller tickets if that will help get code merged.
I agree with this. However, this doesn't mean that untested changes can go in (not that I think you're suggesting otherwise). The serial port case was (and is) challenging because the code had no test coverage at all (#3690 adds a small bit of coverage), so no change was possible without figuring out a testing strategy. Even *manual* testing is challenging - I had a serial device at one port but I can't find the cable anymore. It would have been nice if someone had thought to use the more focused testing approach (that #3690 ended up taking) earlier on in the ticket's life. Jean-Paul