Fixes to make CI green - Githubissues

thecodingmachine / graphqlite-bundle

A Symfony bundle for thecodingmachine/graphqlite.

35 stars 39 forks source link

Fixes to make CI green #96

Closed TamasSzigeti closed 3 years ago

TamasSzigeti commented 3 years ago

Several fixes:

Fix obvious errors in earlier untested PR
Drop redundant phpunit package and use latest of symfony phpunit bridge
Bump phpstan
Bumping minimum symfony version to 4.2 as it has already be broken for 4.1
Ensure minimum 4.3 for some symfony packages – was already broken with lower versions
Fixing stan errors
Bump minimum doctrine annotations for php8 compatibility https://github.com/doctrine/annotations/pull/347
Bump minimum phpdocumentor/type-resolver for php8 compatibility https://github.com/phpDocumentor/TypeResolver/pull/114
Enforce stable deps

None of this should be a breaking change

++ Some of this version bumps should go into the graphqlite package! – I can create a PR there later, but it doesn't hurt to have them here as well

moufmouf commented 3 years ago

Woah, so cool! Thanks a lot for this PR! It seems to me the CI is green. Can I merge this (or is it still WIP?)

TamasSzigeti commented 3 years ago

Woah, so cool! Thanks a lot for this PR! It seems to me the CI is green. Can I merge this (or is it still WIP?)

Perfect timing, I just finished (: Do review it please, though! Some comments/questions:

In the dependency cleanup I tried to keep things as loose as possible and only added constraints where it was already breaking. Maybe another time it would make sense to add even harder constraints e.g. symfony >=4.4 as anything below is already unsupported.
I haven't really looked into coveralls, do you still want to use it?
For now I kept travis.yml for reference but maybe it would make sense to just delete it?

moufmouf commented 3 years ago

Perfect timing, I just finished (: Do review it please, though!

I did review it! Really good job :+1:

Maybe another time it would make sense to add even harder constraints e.g. symfony >=4.4 as anything below is already unsupported.

Definitely, it would make sense for the next minor version!

I haven't really looked into coveralls, do you still want to use it?

That would be awesome. But we can save that for another PR.

For now I kept travis.yml for reference but maybe it would make sense to just delete it?

Let's keep it for now. There was a trick where I was uninstalling the "security bundle" and running tests again to test the behaviour of the graphqlite-bundle when there is no security bundle. We should probably reproduce that at some point.