Benedikt Franke

Results 442 comments of Benedikt Franke

Could you add a failing test case? Not exactly sure where best to put it since multiple directives are concerned, feel free to choose among `WithDirectiveTest`, `OrderByDirectiveDBTest` or `HasManyDirectiveTest`:

I just applied the codestyle fixes through https://github.com/webonyx/graphql-php/pull/1674, please merge the latest `master` before I review.

Instead of performing an additional database query, can we catch the thrown error and wrap it?

Alright. The first step towards implementing this would be to add a test that reproduces the error and create a pull request with it.

We might try to construct the `ArgumentSet` later, when the `$args` have been modified. That would require dealing with untyped and dynamic args, though.

I propose an addition to the docs in https://github.com/nuwave/lighthouse/pull/2575, does this look right?

This would probably also apply to `@delete` when used as an `ArgResolver`. It also calls `$relation->delete()` or `$related::destroy()`.

Would it be simpler to let Laravel handle the fetching by using `destroy()` in all cases?

I am having trouble finding what the spec has to say about this, I was looking through https://spec.graphql.org/October2021/#sec-Subscription. Perhaps you can add a test case for a subscription that returns...

Are we supposed to run php-cs-fixer on the minimum supported PHP version in a project? What am I supposed to do for a library that still supports PHP 7.4, but...