🇦🇹Austria @klausi

Thanks for reporting!

I think you can use PHPStan ignores like this:

class Schema extends BaseMySqlSchema {

  /**
   * {@inheritdoc}
   *
   * @phpstan-ignore-next-line missingType.return 
   */
  public function addField($table, $field, $spec, $keys_new = []) {
    if ...

Can you try if that works? I would like avoid writing special code to detect and ignore PHPStan comments as that would make a lot of our sniffs more complicated.

Downgrading priority to normal, I don't think this is a major issue as you can do this workaround.

Clarified the problem.

Duplicate of 📌 Can we enforce case statements ending in a : instead of a ; Active .

PHP-CS-Fixer is a different system than PHPCS. Coder uses PHPCS sniffs.

Integrating or running PHP-CS-Fixer is a bit out of scope for this Coder issue, but I'm open to review approaches how we could leverage PHP-CS-Fixer as parallel system to PHPCS in Coder.

Hm, the point in 4.x was to have it explicitly as separate class, see the discussion in https://github.com/drupal-graphql/graphql/issues/1301 .

So I would not backport this. For graphql compose I assume you need to start a new git branch anyway? There are some small typing changes that could be incompatible between graphql 4.x and 5.x. Not sure if it is possible to write graphql compose to be compatible with both versions.

The first alpha version was released, please test! https://www.drupal.org/project/graphql/releases/5.0.0-alpha1 →

Merged.

Add 📌 Merge AlterableComposableSchema into ComposableSchema Active

klausi → created an issue.

Looks good, thanks!

Uploading stable patch file for composer patches.

Thanks, uploading stable patch file for composer patches.

Uploading stable patch file for composer patches.

Sorry, this was already done in the dev version of the module.

klausi → created an issue.

Thanks, looks good! Uploading stable patch file for composer patches.

merge request created, uploading stable patch fiel for composer patches.

klausi → created an issue. See original summary → .

klausi → created an issue.

Turns out the remaining LoigicExceptions in code are correct, I don't think we can remove them.

I merged this now - we can do anything that is unclear or could be improved in follow-ups. Better we get this big change out of the way now so that we can work better on other improvements.

Merged, thanks!

@Andriy: I pushed some changes to the pull request, we can improve the AlterableSchemaTest to test the actual AlterableComposableSchema class.

klausi → made their first commit to this issue’s fork.

Thanks, this will be merged soon.

@adamps: Thank you! Can you make a new release?

Currently we have the 4.x-dev version in composer, but Simplenews Scheduler does not like that when there is no version in Simplenews.

Thanks - sorry we don't provide compatibility guarantees for test code, downgrading to normal.

I think we can add those methods, but be aware that we don't have test coverage for this test code, so it could break again in the future. But I think that is ok, it is just test code afterall.

Thanks, uploading stable patch file for composer patches.

Looks good to me, thanks!

Uploading stable patch file for composer patches.

Looks good to me, thanks!

Uploading stable patch file for composer patches.

I also fixed another deprecated warning in the tests, I think this can stay RTBC.

Also uploading stable patch file for composer patches.

klausi → made their first commit to this issue’s fork.

I pushed the empty array as default value instead, please review!

Also attaching stable patch file here for composer patches.

klausi → made their first commit to this issue’s fork.

Merge request updated. Uploading stable patch file here for composer patches.

I think the tests are still missing here.

Thanks, uploading stable patch file for composer patches.

Thanks for reporting!

This happens when you have no blank line after the PHP open tag. Somehow the Squiz standard confuses Drupal file comments with the function comment. But it does not matter, you can fix that by having a blank line before the file comment.

Having a blank line after the PHP open tags is defined in our coding standards, for example https://www.drupal.org/docs/develop/standards/php/php-coding-standards#s... →

So I think there is nothing to fix here.

PHPCS 3.12 was released that contains my fix, I bumped the version in Coder's composer file.

Sure, but those annotation changes are not breaking changes? So we could do the them anytime later as well?

But if you would like to work on them I'm happy to include the conversions!

Updated the merge request. Uploading stbale patch file for composer patches.

thanks, uploading static file for composer patches.

Uploading static patch file for composer patches.

jjchinquist → credited klausi → .

This will be merged soon.

klausi → created an issue.

Unfortunately this is quite a big change, the important thing is to review the interfaces so that they are solid.

Merged into 5.x.

we will do this in 5.x

klausi → created an issue.

klausi → created an issue.

Automated tests passing now.

Now finally done.

Thanks, this will be merged soon.

vrancje → credited klausi → .

klausi → created an issue.

Merged.

klausi → created an issue.

Thanks, I cleaned up the rest of the code and improved the test code.

This will be merged soon.

vrancje → credited klausi → .

There is a deprecation incompatibility with 🐛 Cache collision when multiple servers are using the same schema plugin Active , opened a follow-up merge request.

Hm, not sure I understand the problem ...

graphql_composable is an example module for demonstration purposes, so it should not be scanned if it is not enabled. Maybe we have a bug that uses the schema files from disabled modules? Or you include the example module by accident somehow?

Fixed in 🐛 GraphQL Core module is missing when trying to install the GraphQL module in Drupal 11 Active , thanks for reporting!

Merge train is running, thanks for reporting!

klausi → made their first commit to this issue’s fork.

Alright, added to the merge train! Thanks!

Merging the info file improvement.

Thanks for reporting!

1) There is no graphql_core module. Did you mean graphql? Should be "drush en graphql"

2) The route in the info file is wrong, we can fix that!

klausi → made their first commit to this issue’s fork.

Added to merge train, will be there soon. Thanks!

Thanks, approach looks good!

I'm thinking about if the API addition could break existing installations - I think it will break all old sites that have dataproducer_populate_default_values set to FALSE. We could allow the parameters to be nullable with the "?" operator, which is ugly but should work?

I think you can go ahead and add tests now!

vrancje → credited klausi → .

I see, thanks for clarifying! Then I will close this for now and we can reopen or make a new issue in the future if this becomes relevant.

Thanks - I think we should only do the PHPCS fixes here, to not make the merge request even bigger with out of scope changes.

PHPStan: 📌 Fix the issues reported by phpcs Needs review

Fixing link in issue summary.

Merged.

klausi → created an issue.

Thanks a lot Jonathan for fixing this in 📌 Add new dictionary to cater for common words not in core dictionaries Active .

I don't think there is anything left to do in Coder, so will close this again.

Thanks for the suggestion!

Can you describe in the issue summary why we would need this for Coder? For checking changes of a Coder merge request I can checkout Coder locally and run it? Or what am I missing here, how would you apply Tugboat to test a merge request?

Another restriction is that we develop Coder at Github https://github.com/pfrenssen/coder . So we don't use/run drupal.org merge requests.

Attaching stable patch file for composer patches without the use statements in the meantime.

Thanks, I think the merge request should only change the nullable types compatibility, but I also see use statement changes? Is that on purpose?

I fixed the comments in the pull request, please review.

Uploading stable patch file for composer patches.

klausi → made their first commit to this issue’s fork.

Also uploading stable patch file for composer patches.

klausi → created an issue.

Is this a security issue? If yes please report privately at https://www.drupal.org/project/simplenews/report-security-issue →

Uploading stable patch file for composer patches.

Closing this as duplicate of 🐛 Fix PHP 8.4 implicit nullable deprecation Needs review . Let's continue there!

Uploading stable patch file for composer patches.