Skip to content

PHP 8.4 support #53

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

Merged
merged 6 commits into from
Dec 9, 2024
Merged

PHP 8.4 support #53

merged 6 commits into from
Dec 9, 2024

Conversation

phpfui
Copy link
Contributor

@phpfui phpfui commented Dec 6, 2024

  • Updated composer.json to support PHP 8.0 and higher only
  • Moved to testing under GitHub Actions
  • Ported to newer PHPUnit
  • Added and corrected types for PHP 8.4 support

ToDo:

Unit test need some work, but not sure what to do here as I am not familiar with the code.

*/
public static function save($destination, $config, RequestInterface $request = null)
public static function save(string $destination, $config, ?RequestInterface $request = null) : bool
Copy link
Member

Choose a reason for hiding this comment

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

maybe type config as string|ConfigInterface

{
$this->config['hashNameCallback'] = $callback;

return $this;
Copy link
Member

Choose a reason for hiding this comment

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

not aligned?

{
$this->config['preprocessCallback'] = $callback;

return $this;
Copy link
Member

Choose a reason for hiding this comment

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

align?

{
$this->config['deleteChunksOnSave'] = $delete;

return $this;
Copy link
Member

Choose a reason for hiding this comment

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

code style

/**
* @param Bucket $gridFS storage of the upload (and chunks)
*/
function __construct(Bucket $gridFS)
function __construct(private Bucket $gridFS)
{
parent::__construct();
$this->gridFs = $gridFS;
Copy link
Member

Choose a reason for hiding this comment

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

remove this?

@AidasK
Copy link
Member

AidasK commented Dec 7, 2024

Are these tests passing locally?

@phpfui
Copy link
Contributor Author

phpfui commented Dec 7, 2024

Will fix these issues. Maybe I should set up phpcsfixer and phpstan too.

Tests fail for me locally, you will have to fix, as you know the code better. Not sure how the mocking works.

@phpfui
Copy link
Contributor Author

phpfui commented Dec 7, 2024

I fixed the save() parameter type. Also fixed the formatting with PHP-CS-Fixer. Corrected a bunch of type errors with PHPStan (removed the MongoConfigInterface, as it is not really needed). I also fixed one of the PHPUnit errors. The other two are caused by the mocking, but not exactly sure how to fix those, so you will need to work on them.

I am going to test this in my project. Will push again if I find and fix any issues. I can't test the MongoDB stuff, as I use SQL only.

@phpfui
Copy link
Contributor Author

phpfui commented Dec 7, 2024

My local testing seems to work with no mods needed. This should probably be released as v1.3 or v2.0

@AidasK AidasK merged commit 24ba9ac into flowjs:master Dec 9, 2024
@AidasK
Copy link
Member

AidasK commented Dec 9, 2024

Released as 2.0, thank you for your contribution <3

@phpfui
Copy link
Contributor Author

phpfui commented Dec 9, 2024

Thanks for the prompt attention. Seems PHP 8.4 compatibility is not a high priority for many maintainers.

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.

2 participants