-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Document stubs #13677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document stubs #13677
Conversation
c6c16d2 to
90b2a67
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I did read this in detail. There are some grammatical oddities, but I'm not sure code reviews are a good tool to fix those.
Please check the README on how to auto-format the files.
Thank you for providing some documentation on the frameless calls 🙂
docs/source/miscellaneous/stubs.rst
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should start out explaining what problem stubs solve exactly, along with maybe a simple example of a stub and a function implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try to come up with a better intro.. I also felt that the current one is not very good :D
docs/source/miscellaneous/stubs.rst
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly, it is about not creating a stack frame, but passing arguments to the function directly via the ICALL operands. Hence the name "frameless".
Thank you! I did read this in detail. There are some grammatical oddities, but I'm not sure code reviews are a good tool to fix those.
Feel free to point these out/suggest a better wording, and I'll try to fix them! I wouldn't want to provide docs which scares native speakers.
6011e1d to
cbb3412
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good just some nits
a5e5648 to
1addf64
Compare
I could do quite a bit to make this easier to read — but would it make sense if I spend an hour on it?
@derickr just a gentle ping: will you have capacity on improving the wording in the near future?
Edit for clarity, add some examples, and use foo/bar less
With the help of Derick, the documentation was revamped :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@iluuu1994 If you have followup suggestions, or anything comes to your mind which should be improved, feel free to reach out to me!
The documentation is now live at https://php.github.io/php-src/miscellaneous/stubs.html
Uh oh!
There was an error while loading. Please reload this page.
I'm not a good writer, so feel free to suggest better wording/rephrase the text wherever needed :)