Skip to content

Skeleton comments cleanup#5630

Open
NickSdot wants to merge 3 commits into
php:masterfrom
NickSdot:cleanup-skeleton-comments
Open

Skeleton comments cleanup#5630
NickSdot wants to merge 3 commits into
php:masterfrom
NickSdot:cleanup-skeleton-comments

Conversation

@NickSdot

@NickSdot NickSdot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

As brought up in Discord, this PR is cleaning up a bunch of commented skeleton code all over the docs. For easier review (main diff is collapsed by GitHub) there are three commits:

1) Removes a tonne of commented skeleton blocks
8f29e59

2) Follow up to remove more commented skeleton left-overs
595d303

Some of them were likely meant as comments, however, their information was either redundant with A) the directly following XML element ID, or B) the directly following XML element name. Meaningful or otherwise mandatory comment-like things were not removed.

3) CI style fixes
a79427c


@jordikroon, tagging you because you said PRs are welcome. :)

Edit: based on feedback on Discord I now prepended the commits with [skip-revcheck].

NickSdot added 2 commits June 22, 2026 17:57
Some of them were likely meant as comments, however, their information was either redundant with A) the directly following XML element ID, or B) the directly following XML element name. Meaningful or otherwise mandatory comment-like things were *not* removed
@TimWolla TimWolla requested review from jordikroon and removed request for TimWolla, kamil-tekiela and ndossche June 22, 2026 12:30
@NickSdot NickSdot force-pushed the cleanup-skeleton-comments branch from cb6bf74 to 595d303 Compare June 22, 2026 13:15
@jordikroon

Copy link
Copy Markdown
Member

@NickSdot How did you do these changes? I see some leading whitespaces that you may want to look into.

For example this line here:

 <refsect1 role="usage"> <!-- {{{ -->

Now has a whitespace at the end.

<refsect1 role="usage"> 

As for reviewing it would help to have the section removals in it's own PR. As we are currently at 12k removed lines.

@NickSdot

NickSdot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

How did you do these changes?

@jordikroon Diverse IDE search "" & replace with "". I already noticed the white spaces. Will follow up!

For the separation in different PRs. If you want I can do that! But what's the difference to check the two commits diffs compared to two PRs? Both is removing comment stuff, so I generally think one PR is fine. But again, if you prefer I can split it out into two. Your call. :)

@NickSdot

Copy link
Copy Markdown
Contributor Author

For what it's worth, the first commit are 10,411 removed lines and the second 788.

@kamil-tekiela kamil-tekiela left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, I haven't checked it line by line but I agree with the changes. The second commit removes only the editor navigation braces which I strongly believe should have never been added.

If the CI is green, this can be merged.

@NickSdot

Copy link
Copy Markdown
Contributor Author

If the CI is green, this can be merged.

I'd need to fix the unrelated simpara -> para and others for that.
Do you want this additionally in the same PR, in a separate commit?

@kamil-tekiela

Copy link
Copy Markdown
Member

Separate commit but it can be in this PR. The PR will probably be rebased without squashing.

@NickSdot NickSdot force-pushed the cleanup-skeleton-comments branch 2 times, most recently from ccb86be to a79427c Compare June 22, 2026 17:39
@NickSdot

Copy link
Copy Markdown
Contributor Author

CI is green @jordikroon @kamil-tekiela

@jordikroon

Copy link
Copy Markdown
Member

Impressive! Thank you. If I don't hear anything I will merge this in a few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants