Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[PHP 8.1] Document CURLStringFile #1154

Merged
merged 6 commits into from
Nov 29, 2021
Merged

[PHP 8.1] Document CURLStringFile #1154

merged 6 commits into from
Nov 29, 2021
+267 −0

Conversation

mumumu
Copy link
Member

@mumumu mumumu commented Nov 27, 2021

Added CURLStringFile document based on php/php-src#6456 and its stubs.

@mumumu mumumu added this to the PHP 8.1 milestone Nov 27, 2021
@mumumu mumumu mentioned this pull request Nov 27, 2021
Closed
@mumumu mumumu changed the title Document CURLStringFile [PHP 8.1] Document CURLStringFile Nov 27, 2021
@mumumu
Copy link
Member Author

mumumu commented Nov 27, 2021

I close this PR, because already WIP by CURLStringFile Author.

@mumumu mumumu closed this Nov 27, 2021
@mumumu mumumu deleted the document-curlstringfile-php81 branch November 27, 2021 17:43
@mumumu mumumu restored the document-curlstringfile-php81 branch November 27, 2021 17:52
@mumumu
Copy link
Member Author

mumumu commented Nov 27, 2021

I close this PR, because already WIP by CURLStringFile Author.

Restored after talking with him.　#980 (comment)

@mumumu mumumu reopened this Nov 27, 2021
Irker
Irker reviewed Nov 29, 2021
View reviewed changes
Copy link

@Irker Irker left a comment

Choose a reason for hiding this comment

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

Thank you! Just few not critical comments from me.

reference/curl/curlstringfile/construct.xml Outdated

<refsect1 role="description">
&reftitle.description;
<para>&style.oop;</para>
Copy link

Choose a reason for hiding this comment

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

Do we need this para in case we haven't functional style?

Copy link
Member Author

Choose a reason for hiding this comment

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

Absolutely right! deleted.

reference/curl/curlstringfile/construct.xml Outdated
&reftitle.examples;
<example>
<title><function>CURLStringFile::__construct</function> example</title>
<para>&style.oop;</para>
Copy link

Choose a reason for hiding this comment

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

Do we need this para in case we haven't functional style?

Copy link
Member Author

Choose a reason for hiding this comment

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

ditto.

reference/curl/curlstringfile/construct.xml Outdated
<![CDATA[
<?php
/* http://example.com/upload.php:
<?php var_dump($_FILES); ?>
Copy link

@Irker Irker Nov 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Just an idea: we can dump file name, mime and content. Absolutely not critical

Copy link
Member Author

Choose a reason for hiding this comment

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

We already dumped file_name, mime by var_dump($_FILE);, so added file content in 2bf2139.

reference/curl/curlstringfile.xml Outdated
&reftitle.intro;
<para>
<classname>CURLStringFile</classname> makes it possible to upload a file directly from a variable. This is similar to <classname>CURLFile</classname>, but works with the contents of the file, not filename.
This should be used to upload the contents of the file with <constant>CURLOPT_POSTFIELDS</constant>.
Copy link

Choose a reason for hiding this comment

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

May be "This should be used" -> "This class should be used" ?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed.

@mumumu @cmb69
Update reference/curl/curlstringfile.xml
3e2a5ed 
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
@mumumu mumumu merged commit 27fd6d7 into php:master Nov 29, 2021
@mumumu mumumu deleted the document-curlstringfile-php81 branch November 29, 2021 14:27
@salathe
Copy link
Contributor

salathe commented Nov 29, 2021

Missing a mention of CURLStringFile in the curl_setopt() page, in the description of the CURLOPT_POSTFIELDS option.
Also, the "See Also" sections of curl_setopt(), CURLFile and CURLStringFile should probably all reference each other.

@mumumu
Copy link
Member Author

mumumu commented Nov 29, 2021

Missing a mention of CURLStringFile in the curl_setopt() page, in the description of the CURLOPT_POSTFIELDS option. Also, the "See Also" sections of curl_setopt(), CURLFile and CURLStringFile should probably all reference each other.

Thanks, fixed in 75c0d66.

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

@cmb69 cmb69 cmb69 left review comments

@Irker Irker Irker approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
PHP 8.1
Development

Successfully merging this pull request may close these issues.
5 participants
@mumumu @salathe @Irker @cmb69 @afilina