Skip to content

Promote warnings to errors in dir stdlib extension #4643

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

Closed
wants to merge 2 commits into from

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Aug 28, 2019

Warnings changed:

  • "%d is not a valid Directory resource" promoted to TypeError
  • "Directory name cannot be empty" promoted to Error

@krakjoe
Copy link
Member

krakjoe commented Aug 29, 2019

I'm hesitating because TypeError doesn't necessarily feel right ... it's type is "resource", it has the correct "type" but is the wrong "stream resource type" ...

Anyone else got words ?

@Girgias
Copy link
Member Author

Girgias commented Aug 29, 2019

I'm hesitating because TypeError doesn't necessarily feel right ... it's type is "resource", it has the correct "type" but is the wrong "stream resource type" ...

Anyone else got words ?

I wasn't sure myself, if the consensus is to change it to an Error I'll do that.

@nikic
Copy link
Member

nikic commented Aug 29, 2019

We also use TypeError for incorrect resource types (like expected stream resource got mysqli resource). This seems similar enough to that to me.

@krakjoe
Copy link
Member

krakjoe commented Aug 30, 2019

Merged as b3d7252

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants