- Issue created by @O'Briat
- last update
over 1 year ago 4 pass - @obriat opened merge request.
- last update
over 1 year ago PHPLint Failed - last update
over 1 year ago PHPLint Failed - last update
over 1 year ago PHPLint Failed - last update
over 1 year ago PHPLint Failed - last update
over 1 year ago PHPLint Failed - last update
over 1 year ago PHPLint Failed - last update
over 1 year ago PHPLint Failed - last update
over 1 year ago PHPLint Failed - last update
over 1 year ago PHPLint Failed - Status changed to Needs work
over 1 year ago 6:55pm 1 August 2023 - 🇺🇸United States tr Cascadia
I really don't find this helpful. You have one huge patch that duplicates all the sub issues where specific parts of the drupal-check output is being addressed. If this is meant to be a meta issue, then there should be NO patch, just a collection of links to the open issues. If there are problems that don't have open issues, then you should open new issues for those problems. But throwing all these unrelated problems into one patch is not helpful because all these things have different causes and different solutions.
Your drupal-check output is also wrong - all those "errors" related to the test cases are because of how you've configured your site, not because the test cases are wrong.
drupal-check can be a useful tool to identify POSSIBLE problems - I've used it for years. The goal is NOT to eliminate all drupal-check messages, but to use it as a tool to identify possible problems then examine those possible problems in more detail to see if they really are problems.
If you're going to do this, then please take the time to look into each message, determine why that message is being shown, and if something needs to be changed first find out if there is another issue handling that change. If not, THEN you should open a new issue focused on that one message (or group of similar messages) - that issue should explain what the problem is and why it is a problem and hopefully present a proposed solution. The only thing in your output that I hadn't seen before is the PHP 8.2 issue with dynamic properties. But like all the other problems this doesn't affect the operation of the module.
I'm not going to go through this patch in detail because I've already done that in all the other issues.
As a matter of process, when you submit a patch please set the status to "Needs review", which acts as a flag to indicate that you have proposed a solution with a patch and need others to look at it. "Active" does not indicate that a patch is available.
Likewise, when you submit a patch, please check the DrupalCI automated test output. If your patch fails like the above (PHP 7.3 & MySQL 5.7, D9.5.5 PHPLint Failed) then please figure out why it failed and correct the errors. Here, it's because you're trying to use a PHP feature that isn't supported by all the versions of PHP that Drupal supports.
Also, it is much more useful if you would actually review and comment on the existing issues and fix the problems in those existing issues so they can be committed.
- 🇫🇷France O'Briat Nantes
Thanks for the advices, I'll try to follow them.
My goal was to track minor issues in issue and point to other issues for the major ones.
The status was left to Active as it's a WIP. - last update
over 1 year ago 4 pass - Status changed to Closed: outdated
3 months ago 7:07pm 2 September 2024