When doing docs for block.js (#2502629: JSDoc block module) I found this part that I wanted to "clean up" and talked to nod_ in IRC about it as well.
As pointed out in that issue, the moving of a function is not the same as changing docs, so here is a new issue for that.
Since the patch was already ready, I am just marking this postponed (as the diff relies on changes in above mentioned issue).
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | move_checkemptyregions-2503981-8.patch | 6.06 KB | eiriksm |
| #4 | move_checkemptyregions-2503981-4.patch | 3.02 KB | eiriksm |
| #1 | move_checkemptyregions-2503981-1.patch | 3 KB | eiriksm |
Comments
Comment #1
eiriksmComment #2
eiriksmComment #3
nod_Comment #4
eiriksmRe-roll.
Now not postponed anymore, I guess.
Comment #5
eiriksmComment #7
droplet commentedand updateLastPlaced, updateBlockWeights ?
Comment #8
eiriksmHm good point. Those were probably added after I made the initial patch. Also, preeeety sure this patch did not cause that test failure :p
New patch attached.
Comment #9
droplet commentedThanks.
Comment #11
catchThis is just moving code around and updating the declaration, so looks OK for 8.0.x to me.
Comment #12
catchHowever it's impossible to tell from reading the patch if that's all it does, and we have zero js test coverage, so moving to 8.1.x.
Comment #13
droplet commentedchange in same scope and same function name. No difference in JS. In runtime, JS vars will be hoisting to top, and end up with same way.
(I'm fine it's landed in D8.1 anyway, that's ZERO changes.)
Comment #15
catch@droplet yes there's no actual change, but there's also no way to verify the zero change without line-by-line reviewing the functions. With js test coverage I'd feel a lot happier about making the change, but until we have that, minor releases give a bit more time to let things bed in.
If we later have to fix this JavaScript, and the fact we didn't commit this to 8.0.x causes a merge conflict, we can always re-open this issue and cherry-pick then.
Committed/pushed to 8.1.x, thanks!