Skip to content

Conversation

jderusse
Copy link
Contributor

Similar #61 for parameters

When a parameter is removed (ie. $is_dst in mktime cf http://fr2.php.net/manual/fr/function.mktime.php) calling the method with more parameter than allowed triiger a warning like Error Warning: mktime() expects at most 6 parameters, 7 given

This PR ignore such parameters.

note: In the future if the project supports several major version in parallel, it will have to be able to generate different code for each running version.

function mktime(int $hour = null, int $minute = null, int $second = null, int $month = null, int $day = null, int $year = null): int
{
    error_clear_last();
    if (PHP_VERSION < 70000) {
        $result = \mktime($hour, $minute, $second, $month, $day, $year, $is_dst);
    } else {
        $result = \mktime($hour, $minute, $second, $month, $day, $year);
    }
    if ($result === false) {
        throw DatetimeException::createFromPhpError();
    }
     return $result;
 }

@coveralls
Copy link

coveralls commented Nov 15, 2018

Pull Request Test Coverage Report for Build 128

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 33 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+3.2%) to 34.489%

Files with Coverage Reduction New Missed Lines %
src/Method.php 33 44.94%
Totals Coverage Status
Change from base Build 127: 3.2%
Covered Lines: 179
Relevant Lines: 519

💛 - Coveralls

@moufmouf
Copy link
Member

note: In the future if the project supports several major version in parallel, it will have to be able to generate different code for each running version.

Absolutely. For PHP 8, we can probably create a new minor version with a "require": { "php": ">=8" } constraint and let Composer deal with what version to install based on the user's PHP version.

Thanks a lot for the PR! 💯

@moufmouf moufmouf merged commit 204aef2 into thecodingmachine:master Nov 15, 2018
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.

3 participants