search

glideapps / quicktype

Generate types and converters from JSON, Schema, and GraphQL
https://app.quicktype.io
Apache License 2.0
11.93k stars 1.05k forks source link

Better PHP Support #2470

Open HighLiuk opened 7 months ago

HighLiuk commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

This is amazing!

Our CI already runs PHP tests.

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

dvdsgl commented 7 months ago

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

HighLiuk commented 7 months ago

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 commented 7 months ago

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 commented 6 months ago

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

dvdsgl commented 4 months ago

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

HighLiuk commented 4 months ago

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 commented 4 months ago

Oh! Sorry, I thought the tests were passing in CI and we just needed to update before merge. Thank you for updating anyway.

On Mon, Apr 15, 2024 at 3:55 AM Luca Di Fazio @.***> wrote:

Hey @dvdsgl https://github.com/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.

— Reply to this email directly, view it on GitHub https://github.com/glideapps/quicktype/pull/2470#issuecomment-2056003623, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA2NJKUHYEQNBKJBX53TZTY5OBW5AVCNFSM6AAAAABBXF2PTSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJWGAYDGNRSGM . You are receiving this because you were mentioned.Message ID: @.***>

HighLiuk commented 3 months ago

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?