Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Filesystem] Updated phpdoc on allowed types of content#20980

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
ro0NL wants to merge2 commits intosymfony:2.7fromro0NL:patch-1
Closed

[Filesystem] Updated phpdoc on allowed types of content#20980

ro0NL wants to merge2 commits intosymfony:2.7fromro0NL:patch-1

Conversation

@ro0NL
Copy link
Contributor

QA
Branch?2.7
Bug fix?no
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#20612 (comment)
LicenseMIT
Doc PRsymfony/symfony-docs#...

@chalasr
Copy link
Member

I believedumpFile does not handle streams before 2.8 (see#16156), maybe this should target it instead of 2.7?

Status: reviewed
👍

@ro0NL
Copy link
ContributorAuthor

I think were good... this applies to$content as a stream resource used withfile_put_contents.

@nicolas-grekasnicolas-grekas added this to the2.7 milestoneDec 26, 2016
@nicolas-grekas
Copy link
Member

Technically, this is right, but this is a contract change. I'm not sure we want to guarantee that it works for anything else but strings.
If we want, then this needs corresponding test cases, and it could target master.

@nicolas-grekas
Copy link
Member

That could even be considered a BC break if we read things strictly.

@xabbuh
Copy link
Member

👎 for me. Making this change means that we have to support the other types too. Thus, moving away fromfile_put_contents() in the future could become harder than necessary.

@ro0NL
Copy link
ContributorAuthor

ro0NL commentedDec 28, 2016
edited
Loading

Making it a behavior discussion then.. do we allow users to rely on itright now?

edit: this would lead tois_string safety guard... closing :)

@ro0NLro0NL closed thisDec 28, 2016
@ro0NLro0NL deleted the patch-1 branchDecember 28, 2016 16:41
fabpot added a commit that referenced this pull requestMar 3, 2019
…ays in dumpFile() and appendToFile() (thewilkybarkid)This PR was squashed before being merged into the 4.3-dev branch (closes#29661).Discussion----------[Filesystem] Support resources and deprecate using arrays in dumpFile() and appendToFile()| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | yes| Tests pass?   | yes| Fixed tickets || License       | MIT| Doc PR        |Running PHPStan on my project picked up that passing a resource to `Filesystem::dumpFile()` didn't match the documented type.I found this has been discussed in#20980 and#28019, without a clear result. But, my reading is that only strings should be supported. While I think that not supporting streams makes this a lot less useful (and I'm going to switch away from it), this does need to be resolved. So, I've deprecated using arrays and resources.Commits-------0eaf9d2 [Filesystem] Support resources and deprecate using arrays in dumpFile() and appendToFile()
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

2.7

Development

Successfully merging this pull request may close these issues.

5 participants

@ro0NL@chalasr@nicolas-grekas@xabbuh@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp