Skip to content

Don't urlencode integer values - #895

Merged
acrobat merged 1 commit into
KnpLabs:2.xfrom
acrobat:remove-urlencode-integers
Jul 3, 2020
Merged

Don't urlencode integer values#895
acrobat merged 1 commit into
KnpLabs:2.xfrom
acrobat:remove-urlencode-integers

Conversation

@acrobat

@acrobat acrobat commented Jul 3, 2020

Copy link
Copy Markdown
Collaborator

Also some types have changed to string to be in line with the github api types

@acrobat
acrobat force-pushed the remove-urlencode-integers branch from 8095921 to 2cfb13d Compare July 3, 2020 19:57
@GrahamCampbell

Copy link
Copy Markdown
Contributor

Nice. 👍

@GrahamCampbell

Copy link
Copy Markdown
Contributor

I wonder if we should add strict typing to all the files in 3.x?

@acrobat

acrobat commented Jul 3, 2020

Copy link
Copy Markdown
Collaborator Author

I think that could be a next step indeed. I'm preping a PR to set all docblocks correctly in the 2.x branch, which will allow us to use an automatic tool to convert these to typehints in v3.

@GrahamCampbell

Copy link
Copy Markdown
Contributor

Could be risky if there are mistakes in the doc. I am in the process of doing something similar for the GitLab client https://github.com/m4tthumphrey/php-gitlab-api, however I won't add any scalar types to anything in the Api namespace for fear of breaking stuff. I will, however add strict typing everywhere, and scalar typehints everywhere else in that package. ;)

@acrobat

acrobat commented Jul 3, 2020

Copy link
Copy Markdown
Collaborator Author

Yes, that does mean that the docblocks should be correct. Something we can check but it's not a must for me, if we indeed have typehints in the other parts and enable strict typing it would a step forward!

@acrobat
acrobat merged commit 6a80368 into KnpLabs:2.x Jul 3, 2020
@acrobat
acrobat deleted the remove-urlencode-integers branch July 3, 2020 20:49
@GrahamCampbell

Copy link
Copy Markdown
Contributor

👍

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.

2 participants