Skip to content
This repository has been archived by the owner on Jun 23, 2023. It is now read-only.

PelEntryVersion::setValue raises PHP warning in PHP 7.1 #74

Closed
wants to merge 2 commits into from
Closed

PelEntryVersion::setValue raises PHP warning in PHP 7.1 #74

wants to merge 2 commits into from

Conversation

mondrake
Copy link
Contributor

when the $version parameter passed is non numeric:

A non-numeric value encountered

when the $version parameter passed is non numeric:

_A non-numeric value encountered_
@coveralls
Copy link

coveralls commented Dec 30, 2016

Coverage Status

Coverage increased (+0.2%) to 54.879% when pulling 91f239c on mondrake:patch-1 into 1ec296d on lsolesen:master.

@svenbluege
Copy link
Contributor

My solution for this issue looks like this: svenbluege@1fa9787

What is the best way to fix this?

@weberhofer
Copy link
Collaborator

I think an exception should be thrown in the case of illegal parameters to show the user/developer something is wrong here.

@svenbluege
Copy link
Contributor

This would just change the notice to an exception. So the question is still if we should fix the invalid call to the parent constructor or/and the handling of the data in the PelEntryVersion::setValue method..

@mondrake
Copy link
Contributor Author

mondrake commented Jan 4, 2017

Just note that this is currently occurring within PEL during the file loading process, not (necessarily) in calls from outside code. In other words: PEL raises notices when run in PHP 7.1+. See https://travis-ci.org/lsolesen/pel/jobs/188751705 where the master branch is run against PHP nightly.

This is causing failures also in Drupal testing of the File Metadata Manager module (see #73).

@coveralls
Copy link

coveralls commented Jan 4, 2017

Coverage Status

Coverage increased (+0.02%) to 59.715% when pulling 23d7d36 on mondrake:patch-1 into 8750c5c on lsolesen:master.

weberhofer added a commit that referenced this pull request Jan 4, 2017
* Added PHP 7.1 support
* Do not longer test against PHP 5.5 but PHP 5.6+
* PelEntryVersion does no longer base on PelEntryUndefined but on PelEntry; this makes it working on PHP 7.1 and should speed up the code a little bit (fix for #74)
* Fixed and improved PhpDoc
* Updated phpcs to version 3.0.0beta
@weberhofer
Copy link
Collaborator

Please test the current master version. The issue should be fixed now.

@weberhofer weberhofer closed this Jan 4, 2017
@mondrake
Copy link
Contributor Author

mondrake commented Jan 7, 2017

Confirmed, thank you.

@mondrake mondrake deleted the patch-1 branch January 7, 2017 11:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants