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

Engine to vendor #4392

Merged
merged 3 commits into from Dec 7, 2017

Conversation

Projects
None yet
3 participants
@albertlast
Collaborator

albertlast commented Nov 25, 2017

This PR is a spin off pr: #4391
Since i don't know for sure if this change get merge.

The word "engine" is in mysql context clear defined as storage engine and storage engine are
myisam, innodb, xtradb, blackhole etc.
but this function didn't return the engines it return the vendor of the db -> right name would be vendor.

albertlast added some commits Nov 25, 2017

Handble version_comment better
Signed-off-by: albertlast albertlast@hotmail.de
Change engine to vendor
Signed-off-by: albertlast albertlast@hotmail.de
added missed one
Signed-off-by: albertlast albertlast@hotmail.de
@jdarwood007

This comment has been minimized.

Show comment
Hide comment
@jdarwood007

jdarwood007 Nov 26, 2017

Member

I'm fine with it as long as we don't modify what gets sent up to the SM Stat collection without consulting myself. As I would be the one to fix it on the server side to handle that change.

Member

jdarwood007 commented Nov 26, 2017

I'm fine with it as long as we don't modify what gets sent up to the SM Stat collection without consulting myself. As I would be the one to fix it on the server side to handle that change.

@albertlast

This comment has been minimized.

Show comment
Hide comment
@albertlast

albertlast Nov 26, 2017

Collaborator

@jdarwood007 i never look in the code but when i look in this line https://github.com/albertlast/SMF2.1/blob/654db6417a9b0b61addf029f90bfa05e7867501e/Sources/DbExtra-mysql.php#L417
then the function return fail when the db is a mysql mysql is this wanted?

Collaborator

albertlast commented Nov 26, 2017

@jdarwood007 i never look in the code but when i look in this line https://github.com/albertlast/SMF2.1/blob/654db6417a9b0b61addf029f90bfa05e7867501e/Sources/DbExtra-mysql.php#L417
then the function return fail when the db is a mysql mysql is this wanted?

@jdarwood007

This comment has been minimized.

Show comment
Hide comment
@jdarwood007

jdarwood007 Nov 26, 2017

Member

No that works. If the comment is not empty, it attempts to match. If no match happens the else doesn't happen and it returns with a mysql. If the comment is empty, we return fail.

This could lead to some other fork possibly getting detected as mysql though. I'm not aware of any other major forks at this time though.

Member

jdarwood007 commented Nov 26, 2017

No that works. If the comment is not empty, it attempts to match. If no match happens the else doesn't happen and it returns with a mysql. If the comment is empty, we return fail.

This could lead to some other fork possibly getting detected as mysql though. I'm not aware of any other major forks at this time though.

@colinschoen

LGTM approval

@colinschoen colinschoen merged commit 20ec187 into SimpleMachines:release-2.1 Dec 7, 2017

2 checks passed

Scrutinizer 1 new issues, 1 updated code elements
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@albertlast albertlast deleted the albertlast:EngineToVendor branch Jan 2, 2018

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