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

Upgrade to php 7.1 #616

Merged
merged 6 commits into from
May 31, 2019
Merged

Upgrade to php 7.1 #616

merged 6 commits into from
May 31, 2019

Conversation

Landerstraeten
Copy link
Contributor

No description provided.

@Landerstraeten Landerstraeten requested a review from veewee March 29, 2019 09:08
@Landerstraeten Landerstraeten changed the title Upgrade to php 7.2 Upgrade to php 7.1 Mar 29, 2019
@Landerstraeten Landerstraeten force-pushed the line-nazareth-gent-oostakker branch 2 times, most recently from ad8fab1 to 66b9c87 Compare April 19, 2019 08:10
@Landerstraeten Landerstraeten added this to the 0.15.2 milestone Apr 19, 2019
* @param string|null $value
*/
public function addOptionalArgument(string $argument, $value)
public function addOptionalArgument(string $argument, $value = null): void
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public function addOptionalArgument(string $argument, $value = null): void
public function addOptionalArgument(string $argument, string $value = null): void

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SerkanYildiz $value can also be null or an integer

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, according to the removed php docblock it should be an string|null, that's why it was my suggested change :)

@Landerstraeten Landerstraeten force-pushed the line-nazareth-gent-oostakker branch 4 times, most recently from d749027 to 2c0bd02 Compare May 20, 2019 08:23
@Landerstraeten Landerstraeten force-pushed the line-nazareth-gent-oostakker branch from 2c0bd02 to d0dcdca Compare May 31, 2019 06:09
@veewee veewee merged commit 361c2df into master May 31, 2019
@Landerstraeten Landerstraeten deleted the line-nazareth-gent-oostakker branch June 19, 2019 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants