Skip to content

Document stubs - #13677

Merged
kocsismate merged 10 commits into
php:masterfrom
kocsismate:stub-docs
May 29, 2024
Merged

Document stubs#13677
kocsismate merged 10 commits into
php:masterfrom
kocsismate:stub-docs

Conversation

@kocsismate

@kocsismatekocsismate commented Mar 11, 2024

Copy link
Copy Markdown
Member

I'm not a good writer, so feel free to suggest better wording/rephrase the text wherever needed :)

@kocsismatekocsismate added the Category: Documentation This is for documentation inside php-src, not on php.net label Mar 11, 2024
Comment threaddocs/source/miscellaneous/stubs.rst Outdated
@kocsismate
kocsismateforce-pushed the stub-docs branch 3 times, most recently from c6c16d2 to 90b2a67CompareMarch 11, 2024 22:22

@iluuu1994iluuu1994 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.

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 🙂

Comment threaddocs/source/miscellaneous/stubs.rst Outdated

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.

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.

Copy link
Copy Markdown
MemberAuthor

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

Comment threaddocs/source/miscellaneous/stubs.rst Outdated
Comment threaddocs/source/miscellaneous/stubs.rst Outdated

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.

Mostly, it is about not creating a stack frame, but passing arguments to the function directly via the ICALL operands. Hence the name "frameless".

@kocsismate

Copy link
Copy Markdown
MemberAuthor

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.

@GirgiasGirgias 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.

Mostly looks good just some nits

Comment threaddocs/source/miscellaneous/stubs.rst Outdated
Comment threaddocs/source/miscellaneous/stubs.rst Outdated
Comment threaddocs/source/miscellaneous/stubs.rst
Comment threaddocs/source/miscellaneous/stubs.rst Outdated
Comment threaddocs/source/miscellaneous/stubs.rst Outdated
Comment threaddocs/source/miscellaneous/stubs.rst Outdated
@derickr

Copy link
Copy Markdown
Member

I could do quite a bit to make this easier to read — but would it make sense if I spend an hour on it?

@kocsismate

Copy link
Copy Markdown
MemberAuthor

@derickr just a gentle ping: will you have capacity on improving the wording in the near future?

@kocsismate

Copy link
Copy Markdown
MemberAuthor

With the help of Derick, the documentation was revamped :)

@GirgiasGirgias 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.

LGTM

@kocsismate
kocsismate merged commit 0f64901 into php:masterMay 29, 2024
@kocsismate
kocsismate deleted the stub-docs branch May 29, 2024 12:18
@kocsismate

Copy link
Copy Markdown
MemberAuthor

@iluuu1994 If you have followup suggestions, or anything comes to your mind which should be improved, feel free to reach out to me!

@kocsismate

Copy link
Copy Markdown
MemberAuthor

The documentation is now live at https://php.github.io/php-src/miscellaneous/stubs.html

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

Labels

Category: DocumentationThis is for documentation inside php-src, not on php.net

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@kocsismate@derickr@iluuu1994@Girgias