Skip to content

Update minimum php version to 7.0.0#59

Merged
akrabat merged 5 commits into
slimphp:masterfrom
dbtlr:update-min-php7
Sep 8, 2018
Merged

Update minimum php version to 7.0.0#59
akrabat merged 5 commits into
slimphp:masterfrom
dbtlr:update-min-php7

Conversation

@dbtlr
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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

Comment thread src/Uri.php Outdated
if ($clone->user) {
$clone->password = $password ? $this->filterUserInfo($password) : '';
if ($clone->user !== '') {
$clone->password = $password !== '' ? $this->filterUserInfo($password) : '';
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

akrabat commented Sep 8, 2018

Rebased due to merge of the PRs.

@akrabat
Copy link
Copy Markdown
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
Copy Markdown
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