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

Update minimum php version to 7.0.0 #59

Merged
merged 5 commits into from Sep 8, 2018
Merged

Conversation

dbtlr
Copy link
Contributor

@dbtlr dbtlr commented Sep 7, 2018

Fixes #58

Updates the minimum version requirement to PHP >= 7.0.0.

Also fixes some of the broken unit tests in PHP 7.1 and higher. These were due to two problems:

  1. The way the temp files were created to test uploaded files used tmpfile() which creates a resource, instead of a file name. fopen() no longer allows resources to be passed in. An alternative way to handle this would be to check for a resource and to use it directly, instead of calling fopen(), however we wouldn't be able to guarantee that the w flag was used. Opted to just fix how tests were written to use file names instead.
  2. In URL creation empty checks were using falsy checks, which matched "0" as well. Updated to use exact !== '' checks instead.

@dbtlr dbtlr changed the title Update min php7 Update minimum php version to 7.0.0 Sep 7, 2018
@coveralls
Copy link

coveralls commented Sep 7, 2018

Coverage Status

Coverage remained the same at 97.792% when pulling fadfa3f on dbtlr:update-min-php7 into 3151643 on slimphp:master.

@dbtlr
Copy link
Contributor Author

dbtlr commented Sep 7, 2018

@akrabat Could I get a review/merge on this. It could help move a few other PRs forward as well, including #27

src/Uri.php Outdated
if ($clone->user) {
$clone->password = $password ? $this->filterUserInfo($password) : '';
if ($clone->user !== '') {
$clone->password = $password !== '' ? $this->filterUserInfo($password) : '';
Copy link
Member

Choose a reason for hiding this comment

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

What happens if null is passed in for $password?

@akrabat
Copy link
Member

akrabat commented Sep 8, 2018

I think we shouldn't change the specificity of the conditionals without also type hinting.

@akrabat akrabat mentioned this pull request Sep 8, 2018
@akrabat
Copy link
Member

akrabat commented Sep 8, 2018

Rebased due to merge of the PRs.

@akrabat
Copy link
Member

akrabat commented Sep 8, 2018

Turns out my comments were moot!

akrabat added a commit to akrabat/Slim-Http that referenced this pull request Sep 8, 2018
@akrabat akrabat added this to the 0.4 milestone Sep 8, 2018
@akrabat akrabat merged commit fadfa3f into slimphp:master Sep 8, 2018
@akrabat
Copy link
Member

akrabat commented Sep 8, 2018

Thanks @dbtlr !

@dbtlr dbtlr deleted the update-min-php7 branch September 8, 2018 21:31
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.

Update to require >= PHP 7.0.0
3 participants