Gent
Account created on 10 October 2005, over 19 years ago
#

Merge Requests

More

Recent comments

🇧🇪Belgium svendecabooter Gent

MR looks good to me.
Maybe we also need to add the 'hooks_converted' parameter for performance improvement: https://www.drupal.org/node/3490771

🇧🇪Belgium svendecabooter Gent

Seems like core is mostly putting all hooks together in a Hook\[MODULENAME]Hooks.php file, but I agree splitting up in logical chunks / files makes sense.

🇧🇪Belgium svendecabooter Gent

- Hook help still needs to be added as OOP hook
- All hooks need to be marked as converted, as per https://www.drupal.org/node/3490771

🇧🇪Belgium svendecabooter Gent

Reverted, because module still contains a preprocess procedural hook.

🇧🇪Belgium svendecabooter Gent

The "configuration" section of the readme should describe that the Range Input widget needs to be selected for a relevant facet, and the Range Input processor be selected, for this to work.

The help text in MR #5 does not correctly reflect this.

🇧🇪Belgium svendecabooter Gent

svendecabooter changed the visibility of the branch 3337791-replace-readme.txt-to to hidden.

🇧🇪Belgium svendecabooter Gent

Thanks for the MR. Committed now.

🇧🇪Belgium svendecabooter Gent

Drupal 9 is no longer officially supported, and OOP hooks only work for 10.1+ anyway

🇧🇪Belgium svendecabooter Gent

I could work on this if I find some time. Currently just reporting this, without intent to immediately provide some code.

🇧🇪Belgium svendecabooter Gent

MR looks good.
Still to be added: Setting minimum Drupal version to 10.2

🇧🇪Belgium svendecabooter Gent

MR introduces PHPCS / PHPStan issues, which would ideally be resolved before merging.

🇧🇪Belgium svendecabooter Gent

Gave some feedback on the MR

🇧🇪Belgium svendecabooter Gent

I think it would be better to ignore the PHPStan error about the internal class usage, by ignoring it through a phpstan.neon file in the project, rather than copying over the whole PHP class from core...

Although just ignoring it might give a false sense of security, since we are overriding an internal class, that might suddenly break without BC warnings. But that should probably be covered by test scenario's.

🇧🇪Belgium svendecabooter Gent

Thanks for the review.
Hopefully this can be merged in quickly, to unblock multilingual sites from using la_eu.

🇧🇪Belgium svendecabooter Gent

Thanks for the patch.
It doesn't look like UUID export will be committed to core anytime soon, but in the meantime we can already add support for people using this patches in the referenced issue.

🇧🇪Belgium svendecabooter Gent

Thanks for the report and fix.
We had encountered the same issue in one of our projects, and confirmed this as a solution.

🇧🇪Belgium svendecabooter Gent

svendecabooter made their first commit to this issue’s fork.

🇧🇪Belgium svendecabooter Gent

Added MR + uploaded patch file for Composer-based workflows.

🇧🇪Belgium svendecabooter Gent

I have added test coverage for this.
Not sure if this belongs in LoaderLanguageTest specifically, but it had all the setup logic already available, so seemed like the easiest place to add this test.

🇧🇪Belgium svendecabooter Gent

Logic has evolved too much to fix the MR in this issue.
Closing this one in favor of 📌 Add Gitlab CI integration + fix coding standards issues Active , while giving credit to contributors in this issue.

🇧🇪Belgium svendecabooter Gent

svendecabooter made their first commit to this issue’s fork.

Production build 0.71.5 2024