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.

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

Finish mysql to mysqli conversion #75

Merged
merged 7 commits into from
Jul 20, 2016

Conversation

tuxayo
Copy link
Contributor

@tuxayo tuxayo commented Jul 6, 2016

Look at the diffs commit by commit. Because before doing more than trivial editions to a file, I fixed the coding style on a separate commit so that the diffs are clean (and hopefully easier to review).

Part of #54

}

public function escapeString($value) {
return mysql_real_escape_string($value);
return $this->db->escapeString($value);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

^^^^^^^^^^
That was fixed in a latter commit.
893abdb#diff-5cf75f5d32772b1eb7e42cb21acede85L61

@veggiematts
Copy link
Contributor

I reviewed the code, and everything seems ok for me.

Can someone else have a look at this?

And add spaces before openning curly brackets
the process was the same as:
ndlibersa/resources@26ae17c
(saved on https://archive.is if repo is deleted)
And add spaces before openning curly brackets
The process was the same as:
ndlibersa/resources@26ae17c
(saved on https://archive.is if repo is deleted)

And fix error in management conversion
mysql was fine with: WHERE c.`licenseID`=
whereas mysqli throws a syntax error and needs: WHERE c.`licenseID`=''

Part of coral-erm#54
To avoid regressions because mysql was fine with: WHERE c.`licenseID`=
whereas mysqli throws a syntax error and needs: WHERE c.`licenseID`=''
This is preventive and not in response to observed regressions.
Also it improves consistency as most of Coral's SQL building use this
form for WHERE parameters.

Part of coral-erm#54
@tuxayo tuxayo force-pushed the gh54_mysql_to_mysqli_conversion branch from 9896574 to b047ac5 Compare July 12, 2016 12:04
@jeffnm jeffnm self-assigned this Jul 20, 2016
@jeffnm
Copy link
Member

jeffnm commented Jul 20, 2016

I was able to install the module and create a document. The mysqli database connections seem to be working correctly. I'll go ahead and merge it.

@jeffnm jeffnm merged commit dadbad0 into coral-erm:master Jul 20, 2016
@tuxayo tuxayo deleted the gh54_mysql_to_mysqli_conversion branch July 21, 2016 07:03
@tuxayo tuxayo mentioned this pull request Aug 8, 2016
@PaulPoulain PaulPoulain added this to the Version 2.0.0 milestone Mar 19, 2018
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.

None yet

4 participants