🇦🇹Austria @klausi

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.

Thanks for testing, merged!

I updated your merge request by replacing all the static calls in TaxonomyManagerTree. It is bad practice to repeat the class name all over the place, then any extending class cannot selectively override methods.

I also updated the core version requirement to 10.3.

Let me know if this works for you! I want to make a new release soon.

I don't fully understand what is exactly broken for you - can you post the error message?

The old issue was broken because the annotation "@FormElement" was changed by accident. That was not done here.

Merged.

klausi → created an issue.

Was fixed with 🐛 Restore term clicking AJAX functionality Active .

Merged manually, thanks!

Will fix the broken pipelines in a follow-up.

Thanks, PHPunit build tests are failing, but seems unrelated?

I think this is not possible in PHPCS or Coder, as it does not build a full function reference list of the code base.

PHPStan does that and can also parse comments, so I would suggest to open an issue at https://github.com/mglaman/phpstan-drupal and try to implement it there.

Thank you!

Looking good to me, I think we are ready to commit.

We now have a new approach for the AJAX functionality in 🐛 Restore term clicking AJAX functionality Active that works with multi-value fields and media libraries.

Please test the merge request there. If there are no objections I'm planning to merge it in a couple of days.

Thanks @torfj for working on it so hard!

Nice, looks good to me! Please fix the coding standard issues, otherwise I think we are ready to merge. The CI pipeline is failing, but that is independent of this issue and we can ignore here.

Marking this as RTBC.

@vladimiraus I would wait a couple of days if there are any objections and then merge this.

Hm, but that is a class constant. Of course class constants MUST NOT be prefixed with the module name as that would make them unnecessarily long. Class constants are already tied to a class with namespace, so no prefixing necessary.

The sniff Drupal.Semantics.ConstantName is about legacy global constants in the global scope that must not be used in modern Drupal anymore. (They cannot be autoloaded for example as far as I know)

We could add a new sniff to forbid global constants all together? Would probably make sense to open a new coding standards issue for that if we don't have it yet.

Does that answer your concern? Then I think we can close this.

Thanks!

One issue I found is that when you save a term it redirects you to the term page. But I would expect that I stay in Taxonomy Manager. Should be fixable by setting the correct form state redirect.

I did not test the media library integration - does that work as well that there are no AJAX errors? Then we could avoid introducing a setting to switch on/off Ajax mode as @vladimiraus suggested.

Thank you!

I can haz new release?

I'll get you a drink at Mountaincamp :)

Thanks a lot, nice progress!

Left some more small comments on the PR that we should address.

sutharsan is not maintaining this module anymore. I contacted BarisW to merge this and make a new release.

Looks good, thanks!

Attaching a stable patch file for composer patches.

jjchinquist → credited klausi → .

Updated the approach for Feeds 1.0. We don't even need to overwrite the fetch() method now.

Looks good, thanks!

I will contact @e0ipso if he can merge this and make a new release.

Looks good to me!

I will contact @sutharsan to merge this and make a new release.

Looks good to me!

Looks good to me, thanks!

Thanks! Can you make a new migrate_tools release to get the PHP 8.4 deprecation fix included here out?

@rlhawk thanks for committing this! Can you make a new release?

Here is the patch file for composer patches in the meantime.

Thank you! Here is the patch file for composer patches.

@damienmckenna can you merge and make a new release?

Looks good, thanks!

merge request ready.

klausi → created an issue.

Sent a contact request to manuel.adan if he can merge this and make a new release.

Merged, thanks!

Aha, interesting!

But you cannot make drastic changes on 3.x as that has a stable release already. So you have to publish the rewrite under version 4.x, right?

Created a merge request with a different approach for the 3.x version. Please review!

Patch file attached for composer.

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

Thanks, looks good!

Uploading patch file for composer patches.

https://git.drupalcode.org/project/feeds_http_auth_fetcher/-/merge_reque... now ready for review.

Created https://git.drupalcode.org/project/feeds_http_auth_fetcher/-/merge_reque... , but this is not correct yet. Work in progress.

We should also reroll this against to 2.x branch.

Did not see how we can change the target branch in the pull request?

Merge request created.

klausi → created an issue.

Uploading patch file for composer patches.

merge request created.

Oh my, this change destroyed one of the most important features of taxonomy manager: quickly clicking and navigating between terms. Created a citical bug report at 🐛 Restore term clicking AJAX functionality Active , will post a fix shortly for all AJAX fans.

klausi → created an issue.

Updated pull request.

Here also as static file for composer.patches.json

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

1.5 was released.

merge request created.

klausi → created an issue.

Looks good, thanks bot!

Thank you! Can you make a release? 📌 Are there any plans for a stable module version release? Active