Skip to content

Additional tests for SplFileObject #117

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 4 commits into from
Closed

Additional tests for SplFileObject #117

wants to merge 4 commits into from

Conversation

donnut
Copy link

@donnut donnut commented Jun 24, 2012

New tests created during PZF TestFest, mentored by Rafael Dohms

@pierrejoye
Copy link
Contributor

Tests look good to me. I would suggest you to commit them directly. See the wiki for a PR merge howto:

https://wiki.php.net/vcs/gitworkflow#reviewing_and_closing_pull_requests

@donnut
Copy link
Author

donnut commented Jun 27, 2012

Does this mean I have to apply for a Git-account: http://php.net/git-php.php?

@pierrejoye
Copy link
Contributor

Sorry, I thought Rafael opened this PR, as he has commit access for tests afair.

@donnut
Copy link
Author

donnut commented Jun 27, 2012

Ok, then I'll wait for his responds. Thanks.

'READ_AHEAD',
'SKIP_EMPTY',
'READ_CSV',
);
Copy link
Contributor

Choose a reason for hiding this comment

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

This array is not used anywhere in the test.

print_r(PDO::getAvailableDrivers());
?>
--EXPECT--
Array
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this test assumes only sqlite driver is available?

@smalyshev
Copy link
Contributor

I've merged the SPL ones but not PDO ones (see comment above about it). Please fix and resubmit PDO patch.

@php-pulls
Copy link

Comment on behalf of stas at php.net:

merged

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

Successfully merging this pull request may close these issues.

5 participants