Skip to content
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

Update ChildInterface.php #67

Merged
merged 1 commit into from
Jul 4, 2021
Merged

Conversation

mahmutbayri
Copy link
Contributor

Some reactphp component are not support return type because they support >=5.4. When we use reactphp/filesystem it doesn't work. I think we don't need return type declaration

Incompatiable package
https://github.com/reactphp/filesystem/blob/master/src/ChildProcess/Process.php#L18

Some reactphp component are not support return type because they support >=5.4. When we use reactphp/filesystem it doesn't work. I think we don't need return type declaration 

Incompatiable package
https://github.com/reactphp/filesystem/blob/master/src/ChildProcess/Process.php#L18
@mahmutbayri mahmutbayri requested a review from WyriHaximus as a code owner July 2, 2021 16:23
@mahmutbayri
Copy link
Contributor Author

I use php 7.4. I need to install react/filesystem package. When i add platform config into composer.json gile it works fine now.

    ....,
    "config": {
        "platform": {
            "php": "7.3"
        }
    }
   ...,

@mahmutbayri
Copy link
Contributor Author

When the platform is set to php 7.3, composer installs 1.7.0 version of this package what is too old.

@WyriHaximus do you have any suggestion about how react/filesystem package installs correctly?

Thanks.

public static function create(Messenger $messenger, LoopInterface $loop): void;
public static function create(Messenger $messenger, LoopInterface $loop);
Copy link
Owner

Choose a reason for hiding this comment

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

That void return type hint isn't a bug, and intentional as well. However, if I can get the react/filesystem package to work with it removed. I'm accepting that for now and tag 4.0.0 on this package to get it unblocked. Will then do a 5.0.0 later on with additional changes I've had in mind.

@WyriHaximus WyriHaximus added this to the 4.0.0 milestone Jul 4, 2021
@WyriHaximus WyriHaximus merged commit ae25cf7 into WyriHaximus:master Jul 4, 2021
WyriHaximus added a commit that referenced this pull request Jul 4, 2021
WyriHaximus added a commit that referenced this pull request Jul 4, 2021
WyriHaximus added a commit that referenced this pull request Jul 4, 2021
@WyriHaximus WyriHaximus added bug PHP 🐘 Hypertext Pre Processor labels Jul 4, 2021
@WyriHaximus
Copy link
Owner

v4 has been released and resolved the issue with react/filesystem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PHP 🐘 Hypertext Pre Processor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants