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

Throw proper exception when PHP source is not available #211

Merged
merged 1 commit into from
Sep 30, 2023

Conversation

stloyd
Copy link
Contributor

@stloyd stloyd commented Sep 30, 2023

No description provided.

@crazywhalecc crazywhalecc added the kind/framework Issues related to CLI app framework label Sep 30, 2023
@crazywhalecc
Copy link
Owner

crazywhalecc commented Sep 30, 2023

It is indeed better to have a thrown exception, and we do need to make some changes here.

Because I didn't know where to put this code before and I wrote twice, here is a duplicate code here:

public static function getPHPVersionID(): int

It would be better if you could remove it in this PR and change the libraries and extensions that use this method to $this->builder->getPHPVersionID().

If you can express clear information in the exception here, you may be able to use WrongUsageException directly. In addition, the error of no download should be in initSource, where there should only be a situation where there is no extract.

@crazywhalecc crazywhalecc added need test This PR has not been tested yet, cannot merge now and removed need test This PR has not been tested yet, cannot merge now labels Sep 30, 2023
@crazywhalecc crazywhalecc merged commit 8636f2e into crazywhalecc:main Sep 30, 2023
@stloyd stloyd deleted the missing-php-source branch September 30, 2023 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/framework Issues related to CLI app framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants