Opened 4 months ago
Closed 3 months ago
#48447 closed defect (bug) (fixed)
Update WordPress packages with fixes targeted for 5.3 RC3
Reported by: | youknowriad | Owned by: | |
---|---|---|---|
Milestone: | 5.3 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Editor | Keywords: | has-patch dev-feedback commit |
Focuses: | Cc: | ||
PR Number: |
Attachments (2)
Change History (13)
This ticket was mentioned in Slack in #core-editor by youknowriad. View the logs.
4 months ago
#5
follow-up:
↓ 6
@
4 months ago
@SergeyBiryukov The problem is that these files are automatically synced from the packages. everytime someone will perform npm install && npm run build
it will be reverted. We should fix these in Gutenberg instead.
#6
in reply to:
↑ 5
@
4 months ago
Replying to youknowriad:
The problem is that these files are automatically synced...
Right. Seems it would be good to make few "readability improvements" to latest-comments.php
. Added 48447-readability-fixes.diff as an example here, will open a low-priority issue for Gutenberg too.
Generally having nested function calls with each param on its own line adds a lot of "vertical bloat" and "indentation bloat" :)
These can be avoided by making the code a bit more "wordy" but the result is quite easier to read. Also if ()
statements usually signify a new "block" with its own indentation. Usually better to have empty lines around.
Also, for better readability long ternary operators should probably be if () {} ... else {}
blocks instead (especially when returning from them).
The six level deep nested array in register_block_core_latest_comments()
is also quite "problematic" with some vertical and indentation bloat and uneven/weird white spaces but that's a bug in WPCS that needs fixing upstream (the issue has been open there for over 6 months now!).
#7
@
4 months ago
Wooow, doing npm install
:
updated 10 packages and audited 978269 packages in 22.037s found 0 vulnerabilities
Didn't realize we use nearly 1 million packages!!! :)
[46606] looks good here, +1 to merge to 5.3.
#8
follow-up:
↓ 9
@
4 months ago
These changes all look decent but IMO we should treat these files as "vendors" and not touch them at all in Core.
#9
in reply to:
↑ 8
@
4 months ago
Replying to youknowriad:
Right. Any changes would need to be made upstream (for 5.4). Opened https://github.com/WordPress/gutenberg/issues/18143.
@youknowriad This looks good to me. Feel free to commit to
trunk
. Just needs an additional sign-off before backporting!