- User Since
- Apr 18 2019, 7:59 AM (131 w, 2 d)
Thu, Oct 21
The use of sleep is definitely an issue that should be fixed (should have been months ago but delays), but this stops it from being irritating for a bit while not actually really hiding a bug (IIUC)
I'll queue this on stable to help benefit the 5.9.3 release as well, and try to merge into default.
Wed, Oct 20
Not sure why phab hasn't picked up on it yet, but this has been queued.
This only affects default which is why it's not for stable (just so it's clear it's not a mistake).
This looks good, but it should go on stable (as should the fix), I'll tell you if that doesn't apply.
Since this needs a bit of work, you can also go back to add the docstring I asked in the earlier patch in the same update.
I think a docstring would be good for this function. If this ends up being the only nit in the series it can be done in a follow-up.
Note that this should have been on stable, but I think the rebase will go fine.
Thanks, just in time for 5.9.3 and 6.0, we forgot about it :)
Tue, Oct 19
@joerg.sonnenberger mentioned that the length check isn't needed, so I'll amend my change to remove it.
That looks good, thanks.
Adding my comments on top of Matt's.
Aside from my one comment, the commit message should say "whether unknown/ignored children are recorded or not"
@aalekseyev could you follow-up on the new comments?
Mon, Oct 18
After discussing this in a VC, we'll flip the logic to avoid double negatives
This looks good aside from the small comment I have.
Oh also, (sorry I forgot to say anything) the commit message has a typo in "representation".
This does not apply on top of default sorry, please send a rebase.
Sounds good to me!