Skip to content

Raise supported PHP version to 8.0 and related updates #25

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.

Sign up for GitHub

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

Open
reedy wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Raise supported PHP version to 8.0 and related updates #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .github/workflows/php.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ jobs:
strategy:
matrix:
php-versions:
- "7.2"
- "7.3"
- "7.4"
- "8.0"
- "8.1"
- "8.2"
Expand Down
8 changes: 4 additions & 4 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@
}
},
"require": {
"php": ">=7.2"
"php": ">=8.0"
},
"require-dev": {
"ockcyp/covers-validator": "^1.3.3",
"phpunit/phpunit": "^8.5.13||^9.5.0",
"ockcyp/covers-validator": "^1.7.0",
"phpunit/phpunit": "^9.6.34",
"php-parallel-lint/php-console-highlighter": "^1.0",
"php-parallel-lint/php-parallel-lint": "^1.4",
"friendsofphp/php-cs-fixer": "^3.4",
"friendsofphp/php-cs-fixer": "^3.94",
"brainmaestro/composer-git-hooks": "^2.8"
},
"scripts": {
Expand Down
25 changes: 12 additions & 13 deletions phpunit.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
<phpunit colors="true"
beStrictAboutTestsThatDoNotTestAnything="true"
beStrictAboutOutputDuringTests="true">
<testsuites>
<testsuite name="HOTP PHP Tests">
<directory>./tests</directory>
</testsuite>
</testsuites>
<filter>
<whitelist addUncoveredFilesFromWhitelist="true">
<directory suffix=".php">./src</directory>
</whitelist>
</filter>
<?xml version="1.0"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" colors="true" beStrictAboutTestsThatDoNotTestAnything="true" beStrictAboutOutputDuringTests="true" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
<coverage includeUncoveredFiles="true">
<include>
<directory suffix=".php">./src</directory>
</include>
</coverage>
<testsuites>
<testsuite name="HOTP PHP Tests">
<directory>./tests</directory>
</testsuite>
</testsuites>
</phpunit>
4 changes: 2 additions & 2 deletions src/HOTP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static function generateByCounter(string $key, int $counter): HOTPResult
* @param int|false $timestamp a timestamp to calculate for, defaults to time()
* @return HOTPResult a HOTP Result which can be truncated or output
*/
public static function generateByTime(string $key, int $window, $timestamp = false): HOTPResult
public static function generateByTime(string $key, int $window, int|false $timestamp = false): HOTPResult
{
if (!$timestamp && $timestamp !== 0) {
// @codeCoverageIgnoreStart
Expand All @@ -70,7 +70,7 @@ public static function generateByTime(string $key, int $window, $timestamp = fal
* @param int|false $timestamp a timestamp to calculate for, defaults to time()
* @return HOTPResult[]
*/
public static function generateByTimeWindow(string $key, int $window, int $min = -1, int $max = 1, $timestamp = false): array
public static function generateByTimeWindow(string $key, int $window, int $min = -1, int $max = 1, int|false $timestamp = false): array
Copy link
Copy Markdown
Owner

@thecodedrift thecodedrift Mar 23, 2026

Choose a reason for hiding this comment

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

Quick check. Would this typing change also result in older versions no longer being able to run the library (and would we bump major to do this)?

Copy link
Copy Markdown
Collaborator Author

@reedy reedy Mar 23, 2026

Choose a reason for hiding this comment

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

So yeah, to the first part.

In theory composer should take notice of that PHP requirement, and wouldn't let it be installed on an incompatible environment. Obviously, if they run it with --ignore-platform-reqs well that's on them!

Regarding the second part, that seems to be a case of "depends who you ask".

Bumping the major definitely makes it easier and cleaner, and I did do that in https://github.com/thecodedrift/hotp-php/releases/tag/v2.0.0 but I haven't looked what else may have changed between that and the previous reelase.

But some people do argue that because of the version checking in composer, it shouldn't install an incompatible version, and we're not actually doing an api breaking change (I think) which would usually necessitate doing a major version bump.

I'm certainly in no rush for this to be merged and/or released, it was double checking if/what could change if we dropped those older PHP versions.

Copy link
Copy Markdown
Owner

@thecodedrift thecodedrift Mar 24, 2026

Choose a reason for hiding this comment

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

I think I'm just very used to the node approach to semver, where we bump semver-major whenever we remove engine versions. But, unlike composer, node's engine field is loosely enforced.

I'm comfortable with just bumping minor if the PHP requirement gets enforced.

{
if (!$timestamp && $timestamp !== 0) {
// @codeCoverageIgnoreStart
Expand Down
9 changes: 3 additions & 6 deletions src/HOTPResult.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,11 @@
*/
class HOTPResult
{
protected $hash;
protected $decimal;
protected $hex;
private ?int $decimal = null;
private ?string $hex = null;

public function __construct(string $value)
public function __construct(private string $hash)
{
// store raw
$this->hash = $value;
}

/**
Expand Down