Fix ListBuffer.insertAfter, used in patchInPlace#10416
Fix ListBuffer.insertAfter, used in patchInPlace#10416som-snytt merged 1 commit intoscala:2.13.xfrom
Conversation
|
@SethTisue include in 2.13.11? |
If the new elements are added to the end, `last0` needs to be updated. `insertAfter` is used in `patchInPlace` and `insertAll`, but the bug didn't manifest for `insertAll` because it delegates to `addAll` when adding to the end.
Um, I'm torn. On the one hand, bugs in collections methods are high priority. But on the other hand: the bug is highly specific to a particular collection and a particular not-super-commonly-used method on that collection. And, timingwise, 3.3.0 is out; we've already announced a 2.13.11 release candidate and we're 5 days into the 7 day countdown; and throwing in bugfixes at the last minute is admittedly always tempting, but risky. Because 3.3.0 itself was delayed, 2.12.18 and 2.13.11 are really overdue at this point. I suggest, unhappily, that we hold this one for 2.13.12. |
|
I'm generally very cautious, but considering merging in this case because it's in the standard library and it's very, very low risk. I'm really confident about the fix, the only danger I can imagine is that we were incorrectly relying on the bug somewhere, but that's also very unlikely. Timing-wise, would it have to delay the release? |
Yes — that is, unless you're also willing to skip announcing a new release candidate and resetting the 7 day waiting period. |
|
Right, skipping that would go too far I agree. So I guess the question is whether it's worth delaying the release. Hard to say, the report could have come 3 days later and we wouldn't be thinking about it. OTOH, it came now and we could still include the fix... ⚖️ If noone else speaks up, I'd say it's up to you as the release manager. |
|
You can always say "just one more fix." (This fix has minimal return, so isn't worth any risk. Also, I thought the reporter said they would provide a comprehensive fix?) I'd suggest planning a shorter cycle for what is it 2.13.12. |
Yes, let's shoot for 3 months or so. This time there won't be a Scala 3 minor release affecting the timing. |
Fix ListBuffer.insertAfter, used in patchInPlace
Fix ListBuffer.insertAfter, used in patchInPlace
If the new elements are added to the end,
last0needs to be updated.insertAfteris used inpatchInPlaceandinsertAll, but the bug didn't manifest forinsertAllbecause it delegates toaddAllwhen adding to the end.Fixes scala/bug#12796