Skip to content

Renaming parameter names in ext/sqlite3 #6264

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

Conversation

kamil-tekiela
Copy link
Member

s/ms/milliseconds
s/destination_db/sqlite3 - should it be destination_sqlite3?
s/source_dbname/source_database
s/destination_dbname/destination_database
s/dbname/database
s/param_number/param - same as is in PDO
s/param/bind_var
s/expanded/expand
s/column_number/index

Copy link
Member

@kocsismate kocsismate left a comment

Choose a reason for hiding this comment

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

Can you please convert the parameter names to use camelCase (to match the casing of the methods)?

@kocsismate
Copy link
Member

And I see that there are lots of merge commits in the PR, so I'd suggest that you synchronize your branch with the upstream either by rebasing or by using fast-forward merge. These methods will ensure there are no unnecessary commits in your PR.

@kamil-tekiela kamil-tekiela force-pushed the renaming-parameter-names branch 2 times, most recently from 4afea87 to 5fedc75 Compare October 2, 2020 23:26
Update sqlite3_arginfo.h

s/enableExceptions/enable

BindValue uses $value

$name is more appropriate
Camel-casing
Update sqlite3_33_createAggregate_notcallable.phpt
Co-authored-by: Nikita Popov <[email protected]>
s/$argumentCount/$argCount
@kamil-tekiela kamil-tekiela force-pushed the renaming-parameter-names branch from 45e3a4f to cbd638f Compare October 6, 2020 11:36
@kamil-tekiela kamil-tekiela requested a review from nikic October 6, 2020 11:39
Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

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

Looks good from my side. @kocsismate?

@php-pulls php-pulls closed this in 1c411ed Oct 6, 2020
@kamil-tekiela kamil-tekiela deleted the renaming-parameter-names branch October 9, 2020 01:12
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.

4 participants