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

Add support for MySQLi over SSL, fixes #152 #229

Closed
wants to merge 12 commits into
base: develop
from

Conversation

Projects
None yet
3 participants
@machek

machek commented Mar 8, 2017

No description provided.

Zdenek Machek added some commits Mar 8, 2017

Zdenek Machek
Zdenek Machek
Zdenek Machek
Zdenek Machek

@machek machek changed the title from Add support for SSL, fixes #152 to Add support for MySQLi over SSL, fixes #152 Mar 9, 2017

Zdenek Machek added some commits Mar 9, 2017

Zdenek Machek
Zdenek Machek
Zdenek Machek
Zdenek Machek
Zdenek Machek
@froschdesign

This comment has been minimized.

Show comment
Hide comment
@froschdesign

froschdesign Mar 9, 2017

Member

@machek
Btw. please complete your changes and after this you can create a PR. Adding permanently new commits doesn't help.

Member

froschdesign commented Mar 9, 2017

@machek
Btw. please complete your changes and after this you can create a PR. Adding permanently new commits doesn't help.

@froschdesign

This comment has been minimized.

Show comment
Hide comment
@froschdesign

froschdesign Mar 9, 2017

Member

@machek
If you need the test results from Travis, then you can activate Travis for your own fork.

Member

froschdesign commented Mar 9, 2017

@machek
If you need the test results from Travis, then you can activate Travis for your own fork.

Zdenek Machek
@machek

This comment has been minimized.

Show comment
Hide comment
@machek

machek Mar 9, 2017

@froschdesign sorry about too many commits,Travis behaviour was a bit unexpected, I think would be good to enable integration tests for Mysql, db is available so why not.
Otherwise PR is ready for the review now.
Thanks

machek commented Mar 9, 2017

@froschdesign sorry about too many commits,Travis behaviour was a bit unexpected, I think would be good to enable integration tests for Mysql, db is available so why not.
Otherwise PR is ready for the review now.
Thanks

Zdenek Machek
@froschdesign

This comment has been minimized.

Show comment
Hide comment
@froschdesign

froschdesign Mar 9, 2017

Member

@machek

…I think would be good to enable integration tests for Mysql, db is available so why not.

Sure, but this was not my intention. I mean you can test with Travis at your own fork and prevent permanently new commits at a pull requests.

Member

froschdesign commented Mar 9, 2017

@machek

…I think would be good to enable integration tests for Mysql, db is available so why not.

Sure, but this was not my intention. I mean you can test with Travis at your own fork and prevent permanently new commits at a pull requests.

@ezimuel

This comment has been minimized.

Show comment
Hide comment
@ezimuel

ezimuel Nov 23, 2017

Member

@froschdesign what's the status of this PR? I'm working to release zend-db 2.9 and I would like to have this PR included.

Member

ezimuel commented Nov 23, 2017

@froschdesign what's the status of this PR? I'm working to release zend-db 2.9 and I would like to have this PR included.

@ezimuel ezimuel added this to the 2.9.0 milestone Nov 23, 2017

@machek

This comment has been minimized.

Show comment
Hide comment
@machek

machek Nov 23, 2017

I should mention that coverage/coveralls fails because of

    if (!getenv('TESTS_ZEND_DB_ADAPTER_DRIVER_MYSQL')) {
           $this->markTestSkipped('Mysqli integration test disabled');

added unit tests are using mysqli adapter because of nature of these changes
but if you feel that some polishing / changes are required please provide feedback, I am happy look into it once more.

machek commented Nov 23, 2017

I should mention that coverage/coveralls fails because of

    if (!getenv('TESTS_ZEND_DB_ADAPTER_DRIVER_MYSQL')) {
           $this->markTestSkipped('Mysqli integration test disabled');

added unit tests are using mysqli adapter because of nature of these changes
but if you feel that some polishing / changes are required please provide feedback, I am happy look into it once more.

@ezimuel

This comment has been minimized.

Show comment
Hide comment
@ezimuel

ezimuel Nov 28, 2017

Member

Merged in master and develop. Rebased for master, since this PR was against develop.
@machek thanks for your contribution!

Member

ezimuel commented Nov 28, 2017

Merged in master and develop. Rebased for master, since this PR was against develop.
@machek thanks for your contribution!

@ezimuel ezimuel closed this Nov 28, 2017

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