Generate ext/dom class synopses based on stubs#839
Generate ext/dom class synopses based on stubs#839kocsismate wants to merge 2 commits intophp:masterfrom
Conversation
|
@salathe Can you please clarify your concerns? |
|
On initially visually scanning the changes, I stopped reading when the very first file in the list ( At the time, I didn't have any particular desire to continue on down that rabbit hole, so just gave a thumbs down and moved on. I didn't reply to Máté, because I don't want to clarify my concerns. And now, I continue to not have any particular desire to go down this rabbit hole, and so won't be giving a review at this time either. |
|
A problem with reviewing this PR is that it is noisy (particularly the re-ordering of the properties). OTOH, it is done (based on?) an automated script, so later changes won't probably shuffle again. Still, a lot to review. |
Yes, the command I used is I agree that the property reordering makes the PR noisy. We could change the order of properties in php-src, but I'm not sure which order makes more sense, so I'm eager for suggestions. The only thing I'd like to avoid is to make manual adjustments to the generated output, since it would make subsequent class synopsis page regenerations very cumbersome, while otherwise we could syncronize the docs with future php-src changes basically by running a simple command. |
|
This LGTM, I think its more likely the manual was mistaken than the stubs but it might just be wise to double checked all the |
salathe
left a comment
There was a problem hiding this comment.
I managed to get past the first file this time (to 6/20 files), and there seem to be a few issues common to many files in this PR.
See inline comments for specific details.
General note
Reviewing this PR critically has been made more difficult than it really ought to be, due to the mixing of:
a) whitespace/formatting-only changes
b) consistency changes (e.g. adding xi:fallbacks)
c) re-ordering of lists of things (e.g. classref attributes, lists of things)
d) actual content changes (e.g. new interfaces, properties, etc.)
I have very likely missed issues in the latter (let's be honest, most important) category of changes due to the others, even in the few files I've looked through thus far. Also, if it's not clear already, please consider this review incomplete. I encourage other reviewers to cast a critial eye on these changes.
When should I add |
No issues per se, to my knowledge, but they can make reading the diffs harder. |
Generated via #839