Opened 5 years ago
Closed 5 years ago
#4758 closed defect (bug) (fixed)
Adjust function signature of WPorg_Handbook_Walker::walk()
Reported by: | jrf | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | Priority: | high | |
Component: | Handbooks | Keywords: | has-patch |
Cc: |
Description
Currently, one of the last patches for WP 5.3 is blocked by a change which needs to be made in meta.
See: https://core.trac.wordpress.org/ticket/47678#comment:72
The change itself is rather simple - basically the same changes as shown in this patch: https://core.trac.wordpress.org/attachment/ticket/47678/47678-Walker-walk.patch
Unfortunately I don't have access to meta, so can't prepare a patch.
The change needs to be made in this file:
https://meta.trac.wordpress.org/browser/sites/trunk/wordpress.org/public_html/wp-content/plugins/handbook/inc/walker.php#L31
Change History (6)
This ticket was mentioned in Slack in #core-committers by jrf. View the logs.
5 years ago
#3
@
5 years ago
@Otto42 Thanks, but no.
The change originally went in and was reverted as it caused a lot of warnings to be thrown for dotorg.
Commit: https://core.trac.wordpress.org/changeset/45624
Revert: https://core.trac.wordpress.org/changeset/45640
It has been agreed that the change should go into WP 5.3, but to prevent the same load of warnings being thrown, the #meta change should be made at the same time.
In addition, looking at the code change proposed, I don't really understand it.
May I suggest educating yourself by reading the original ticket ?
#4
@
5 years ago
You're kind of missing the point here, I feel.
Yes, a change to core will break org, and thus we correct org afterwards to correct the problem. This happens whenever you break backward compatibility.
But that works either way. If we change the code on .org before changing core, then we get the same problems and warnings. You don't solve a compatibility break by changing it in one place over another. Both changes need to happen simultaneously.
May I suggest educating yourself by reading the original ticket ?
May I suggest that you simply explain the issue in detail here in this ticket, instead of pointing to a 3 month old ticket with 70+ comments and 50+ patch attachments?
Because I'm not going to make this change without some explanations. If this means core is blocked, then core will stay blocked.
#5
@
5 years ago
If we change the code on .org before changing core, then we get the same problems and warnings.
Not true. The change to #meta is BC-compatible with older WP versions (pre the core patch going in).
May I suggest that you simply explain the issue in detail here in this ticket, instead of pointing to a 3 month old ticket with 70+ comments and 50+ patch attachments?
Well, without repeating myself overly much, please read this, which is the draft dev-note and explains both the changes as well as why changing #meta first won't cause an issue: https://gist.github.com/jrfnl/23fa057e5a951ab8b6bf8709e5e5b281
Seems like you have this the wrong way around here.
This piece is a plugin. We don't change plugins before we change core. This change cannot go in until core makes its changes accordingly. Now, we might have to break wp.org to do it, but core changes come before meta changes.
The other way around makes no sense, and it also makes no sense that core changes can be blocked by meta changes, especially ones to the handbook plugin, of all things.
In addition, looking at the code change proposed, I don't really understand it. The syntax must be some of that newfangled PHP 7 stuff or something, but what is the purpose of adding
...$args
instead of just usingfunc_get_args()
? The func_get_args function isn't deprecated or removed. This whole thing seems like an unnecessary change that breaks backward compatibility for no reason or benefit.