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

Better PHP Support #2470

Open
wants to merge 46 commits into
base: master
Choose a base branch
from
Open

Better PHP Support #2470

wants to merge 46 commits into from

Conversation

HighLiuk
Copy link

Would close #2432.

Adds several options & revisits how validation works.
Also, it adds a "PHP Version" option to target a specific version of PHP, turning off features that were not supported in that version automatically. More on this on the related issue.

@HighLiuk
Copy link
Author

@dvdsgl here we are. Question: how is Quicktype supposed to run test against several languages specific code? I'd like to add PHP specific tests (parsing would be nice to start with, but would be nice to run PHP tests on the generated classes if that's possible).

@dvdsgl
Copy link
Member

dvdsgl commented Jan 11, 2024

This is amazing!

Our CI already runs PHP tests.

Take a look at test/languages.ts – search for PHP

@dvdsgl
Copy link
Member

dvdsgl commented Jan 11, 2024

Here's the PHP CI test run: https://github.com/glideapps/quicktype/actions/runs/7494968972/job/20406252954?pr=2470

@HighLiuk
Copy link
Author

Thanks @dvdsgl I'll check them out and update the PR as soon as more tests pass

By the way, tests are running on PHP 8.2 in CI, but if I target different PHP versions, it would be nicer if I can run tests for each PHP version.
Also, last PHP version is 8.3 since a couple weeks

@dvdsgl
Copy link
Member

dvdsgl commented Jan 12, 2024

Sure thing, we can configure the system to run on multiple versions of PHP. I'll just warn you right now that our CI is in a weird state.

@dvdsgl
Copy link
Member

dvdsgl commented Feb 14, 2024

Hi there. Our CI is now fixed – please rebase.

@dvdsgl dvdsgl enabled auto-merge (squash) April 13, 2024 21:50
@dvdsgl dvdsgl disabled auto-merge April 13, 2024 21:50
@dvdsgl
Copy link
Member

dvdsgl commented Apr 13, 2024

@HighLiuk I'd love to merge this—can you rebase so I can run the CI tests?

@HighLiuk
Copy link
Author

HighLiuk commented Apr 15, 2024

Hey @dvdsgl the fact is that I've found on my machine that tests don't pass. So I was diving deeper and then just had other things to do and not so much time to make the tests pass... though I know it's a requirement.

Anyway let's check them out together, I've rebased it (I guess)

@dvdsgl
Copy link
Member

dvdsgl commented Apr 15, 2024 via email

@HighLiuk
Copy link
Author

Hey @dvdsgl, do you have some easy trick to easily test changes in local development (like Nodemon/Vite), without building all the way down every single time I make a change?

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.

Better PHP Support
2 participants