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

Frozen - Lint doesn't support PHP 8.1 #324

Open
dktapps opened this issue May 31, 2023 · 17 comments
Open

Frozen - Lint doesn't support PHP 8.1 #324

dktapps opened this issue May 31, 2023 · 17 comments

Comments

@dktapps
Copy link
Member

dktapps commented May 31, 2023

and probably PHPStan doesn't either

@JaxkDev
Copy link
Member

JaxkDev commented May 31, 2023

Lint is poggits container php version, currently 8.0.13(13 I think)

PHPStan is its own thing, which also needs updating to satisfy composer platform req

@SOF3 I can’t do either of them, you’ll have to

@dktapps
Copy link
Member Author

dktapps commented May 31, 2023

What about running lint inside the PHPStan container? The PHPStan container is anyway easier to work with

@JaxkDev
Copy link
Member

JaxkDev commented May 31, 2023

The containers aren’t exactly ‘stable’ errors like these are very common https://discord.com/channels/402639859535052811/406714907896315915/1112927007731351693

@dktapps
Copy link
Member Author

dktapps commented May 31, 2023

That's just because of OOM on the server. In any case the lint can be run before running PHPStan.

@SOF3
Copy link
Member

SOF3 commented Jun 1, 2023

I would definitely prefer containerizing all jobs and lints. It is just a bit high development cost, because the current code heavily couples with the database, and I don't want to expose database privileges, discord webhooks, etc to job containers. The task of abstracting out these things from the ProjectBuilder infra would be a bit heavy.

@dktapps
Copy link
Member Author

dktapps commented Jun 1, 2023

What about just the syntax lint? The remaining lints should be achievable inside Poggit main as long as the native PHP tokenizer isn't being used.

@SOF3
Copy link
Member

SOF3 commented Jun 1, 2023

iirc the PSR lint uses the native PHP tokenizer too. But yes, we could probably delegate that call to a remote container service, which would be pretty trivial to implement (just a 20-line nodejs microservice).

@dktapps
Copy link
Member Author

dktapps commented Jun 1, 2023

yes, or use some library like PHP-Parser to parse it

@JaxkDev
Copy link
Member

JaxkDev commented Jun 26, 2023

What lints are actually required now?

The way I'm looking at this right now is that PHPStan would be seeing most if not all issues.

@SOF3
Copy link
Member

SOF3 commented Jun 27, 2023

The only real requirement right now is to run a microservice like this

FROM php:8.2-apache
ADD token.php /var/www/html/token.php
<?php // token.php
$tokens = token_get_all(file_get_contents("php://input"));
foreach($tokens as &$token) {
    if(is_array($token)) $token[0] = token_name($token[0]);
}
echo json_encode($tokens);

and change all Poggit calls to token_get_all($code) into json_decode(Curl::curlPost("http://token/token.php", $code))).

@JaxkDev
Copy link
Member

JaxkDev commented Jun 27, 2023

Since this is related to the specific docker setups we have, and not open source I can't commit this etc.

@dktapps
Copy link
Member Author

dktapps commented Jun 27, 2023

We really ought to open source the docker stuff

@SOF3
Copy link
Member

SOF3 commented Jun 28, 2023

Open-sourcing the docker stuff isn't enough. To make Poggit actually usable externally we need to provide a proper base database image too. Right now we only have init script for the tables.

@dktapps
Copy link
Member Author

dktapps commented Jun 28, 2023

are you saying the table init isn't done in a Dockerfile?

@JaxkDev
Copy link
Member

JaxkDev commented Jun 28, 2023

are you saying the table init isn't done in a Dockerfile?

It is, however poggit needs 'some' data (forgot what exactly) before it can actually function and thats not provided either.

@JaxkDev
Copy link
Member

JaxkDev commented May 28, 2024

TLDR; Frozen issue because can't commit new microservices to replace old php tokenizer due to original docker setup not being hosted on github.

@JaxkDev JaxkDev changed the title Lint doesn't support PHP 8.1 [Frozen] Lint doesn't support PHP 8.1 May 28, 2024
@JaxkDev JaxkDev changed the title [Frozen] Lint doesn't support PHP 8.1 Frozen - Lint doesn't support PHP 8.1 May 28, 2024
@SOF3
Copy link
Member

SOF3 commented May 29, 2024

well we could commit the separate service and expose a config to set the endpoint, just like mysql.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants