Skip to content

Use camel case for parameter names in ibm_db2.stub.php #33

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 1 commit into from

Conversation

phansys
Copy link
Contributor

@phansys phansys commented Feb 20, 2023

The parameter $colnum in db2_lob_read() was also renamed to $columnNumber;

TODO:

@phansys phansys marked this pull request as ready for review February 20, 2023 22:43
Copy link
Member

@tessus tessus left a comment

Choose a reason for hiding this comment

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

I think we always used underscores in ibmdb2. But in reality it doesn't matter as long as it is consistent. I defer judgement to the devs who took this over.

@phansys
Copy link
Contributor Author

phansys commented Feb 20, 2023

I understand. I'm proposing this change because I saw the camel case as the most used style in modern PHP libraries. Also, it is already used in other extensions (like PDO, by instance).
By other hand, the casing is actually different between the base (English) documentation and other translations:

@NattyNarwhal
Copy link
Member

I'm also fine with doing whatever as long as it's consistent.

@phansys
Copy link
Contributor Author

phansys commented Feb 24, 2023

I decided to close this PR by now, as I confirmed that the stub generates the proper documentation using underscores. The lack of consistency between translations seems to be a human mistake.
Given that there are many tools relying on the names of the current parameters, I think there is no great gain with this change.

Thank you!

@phansys phansys closed this Feb 24, 2023
@phansys phansys deleted the stub_params branch February 24, 2023 17:01
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.

3 participants