🇺🇸United States @smustgrave

smustgrave → created an issue.

Think we should merge and see if it works.

Re-ran pipeline and all green. Seems pretty straightforward fix.

Going to go on a limb here

Crediting nod_ for suggestion bigPipe which turned out to be it. Don't have a fix but uninstalling that fixes everything.

Rebased

Left comments on the MR.

Will say we have 1000s of tickets that go back 15+ years, it's not that they're ignored it's just how much effort is being put into it. Just briefly looking seems this was re-rolled a bunch but not pushed forward.

✨ Add "Disable image resize" setting to image fields Needs review may be a good example of test.

Seems like a straight forward

Thanks for working on this but issues should be in MRs vs patches and will need test coverage.

Have not reviewed but was previously tagged for tests and fixes should be in MRs vs patches

For all the help getting this far

100% all our plugins need it

Does removing something from the status report page require product manager review?

Rebase seems good.

Needs manual rebase.

As mentioned there are a number of these floating around if you want to focus on one and ping that to me I'll prioritize it @quietone, so you don't have to keep rebasing a dozen of them.

1) Drupal\Tests\system\Functional\Form\ElementTest::testFormElements
Behat\Mink\Exception\ExpectationException: 0 elements matching xpath "//input[@type="button"]" found on the page, but should be 1.
/builds/issue/drupal-289240/vendor/behat/mink/src/WebAssert.php:888
/builds/issue/drupal-289240/vendor/behat/mink/src/WebAssert.php:441
/builds/issue/drupal-289240/core/modules/system/tests/src/Functional/Form/ElementTest.php:161
/builds/issue/drupal-289240/core/modules/system/tests/src/Functional/Form/ElementTest.php:35
FAILURES!
Tests: 1, Assertions: 74, Failures: 1.
Exiting with EXIT_CODE=1

Shows the test coverage

Also resolved the threads because they appeared to all be addressed.

I did not find anything else pending and code looks fine to me.

Going to mark it.

Believe feedback has been addressed for this one.

Do not have an answer for the remaining tasks but the MR appears to need a manual rebase.

Left some comments on the MR.

I've definitely seen this issue before. Especially when I was working on that body auto-create removal ticket and getting a number of failures.

Don't see any issue with using array_unique (makes sense). Will go on a limb and mark this one.

So before working on this more is this heading in the right direction? Is it something that will be accepted?

This something that should have some test coverage? Or a follow up for recipe performance tests?

Re-solved the threads but appears to need a manual rebase (200 commits back), surprised the bot never picked it up.

That does seem worth it, not 100% I can make the call but maybe @longwave?

Also the current MR needs a manual rebase.

Seems like a good re-roll for 10.5

Should this be postponed or is there a larger gain to be add with moving it out?

Believe needs a manual rebase now, probably would update the deprecations for 11.2 now :(

https://git.drupalcode.org/issue/drupal-3440501/-/jobs/3073737 appears to show both test changes are failing as expected.
I checked the schema

    notification:
      type: mapping
      label: 'Notification settings'
      mapping:
        emails:
          type: sequence
          label: 'Email addresses to notify when updates are available'
          sequence:
            type: email
            label: 'Email'

And this appears correct for the change to be able to send multiple emails.

All threads appear to be resolved and feedback I believe addressed

LGTM.

but without the re-save the problem isn't fully fixed though?

test-only feature passes when it should be failing so test coverage needs to be expanded.

MR should be updated to 11.x

But if re-saving is needed to get to work then probably would need an update_hook, using batch API

Believe upgrade path is still needed.

Also deprecations probably have to be bumped to 11.2.x

Think I see what @mstrelan and @longwave are talking about now. Anyway to get the results https://git.drupalcode.org/issue/drupal-3484966/-/pipelines/335642 to the rest of the tests?

How come PhpUnitCliTest is removed?

Believe feedback has been addressed on this one.

Comment on MR.

@dimitriskr sorry haven't had a chance to get to this sooner. I believe we have probably missed the 11.1x window mind updating those for 11.2.x please

I'd assign the issue to you if I could

Thanks for reporting

Think next step will be to add test coverage showing the problem and that the change fixes it.

Think this change I'm only going to include in 3.1.x for now.

Not able to replicate

I'm taking my previous statement for tests back, sorta. 🌱 Add better test coverage per check Active is open. Don't think I should hold a fix for something the module is missing as a whole.

smustgrave → created an issue.

Confirmed the issue and that it addressed the issue. Carried it to 3.0.x also.

Rebase seems good.

Feedback appears to be addressed.

Feedback appears to be addressed, very neat.

Needs a rebase.

There appear to be a lot of these type of tickets floating around if you want to focus on one just ping it to me and I'll prioritize it.

Previously tagged for test which still appear to be needed.

Issue summary also appears to be incomplete.

Failure was random, all green after re-running test.

Change seems small enough and matches the summary I don't mind marking now.

Still appears to be missing proposed solution section.

The one I was curious about was 43.3.0 with media-embed but now realize we don't have that button lol.

Applied the update locally at least and ckeditor seems to behave as expected.

Appears to have some open threads

Also tagging for a change record.

I applied the MR for 11.x and the cspell commands appear to work as expected. Assuming the word changes are either the file out of sync or maybe cspell updated their internal dictionaries?

Very neat!

Left some comments on the MR.

Small change but possible to get test coverage for it.

Also if MR can be updated to 11.x vs 11.1.x

Possible to get simple test coverage to make sure this change catches what we think.

Think we will have to do the backwards compatibility dance.

Wonder if while we are at it we should use constructor promotion so all the params are correct typehinted.

Just to be safe though I wonder if we should add a simple deprecation that triggers if someone passes in the old type. Just to be safe.

So the output is slightly different now but I think for the better

before

  <file name="/var/www/html/web/core/misc/dialog/off-canvas/css/titlebar.pcss.css"></file>
  <file name="/var/www/html/web/core/misc/dialog/off-canvas/css/utility.css"></file>
  <file name="/var/www/html/web/core/misc/dialog/off-canvas/css/utility.pcss.css"></file>
  <file name="/var/www/html/web/core/misc/dialog/off-canvas/css/wrapper.css"></file>
  <file name="/var/www/html/web/core/misc/dialog/off-canvas/css/wrapper.pcss.css"></file>
  <file name="/var/www/html/web/core/themes/claro/css/base/elements.css"></file>
  <file name="/var/www/html/web/core/themes/claro/css/base/elements.pcss.css">
    <error source="stylelint.rules.order/properties-order" line="7" column="3" severity="error" message="Expected &quot;font-family&quot; to come before &quot;font-style&quot; (order/properties-order)" />
  </file>
  <file name="/var/www/html/web/core/themes/claro/css/base/print.css"></file>
  <file name="/var/www/html/web/core/themes/claro/css/base/print.pcss.css"></file>

After

> lint:css-checkstyle
> stylelint "**/*.css" --custom-formatter=@gitlab-formatters/stylelint-formatter-gitlab --output-file=gl-codequality.json

[{"type":"issue","check_name":"order/properties-order","description":"Expected \"font-family\" to come before \"line-height\" (order/properties-order)","content":{"body":"Error found in order/properties-order."},"categories":["Style"],"location":{"path":"themes/claro/css/base/elements.pcss.css","lines":{"begin":7,"end":7},"positions":{"begin":{"line":7,"column":3},"end":{"line":7,"column":35}}},"s

But more importantly it found the error I introduced.

Thanks for reporting, we could use a test case showing this as a problem.

Do we add test coverage for new features added?

Rebase seems fine.

Appears to be open thread in MR, proposed solution should be tweaked some too.

smustgrave → created an issue.

Thanks @kristen pol!

Much appreciated! The project we just took I’m planning for 8.3 for D11 but trying to get their modules on supported levels too

Seems 2.5.x covers d10 fully

Just reopening if either an older branch can be marked supported or D10 dropped since D10 requirements don’t work. Not sure a module can be said D10 compatible if it can’t work on the minimum requirements

That has 8.2 requirement so really no branch is inline with D10 requirements unfortunately

And just FYI I'm not arguing for or against any of these, but am arguing for consistency. If we postpone issues pending a rule being added then we should do that for all. If we allow these in then any of those issues I believe should be un-postponed and eligible to be merged too.

@dburiak mind rebasing

Thanks for reporting,

Seems like an issue that will need test coverage for sure.

Going to say if these get merged then issues like https://www.drupal.org/project/drupal/issues/3486996 🐛 Typo in WidgetInterface.php Active should be unpostponed too

Credited larowlan for triaging 🐛 Link Active

Since it was requested for steps to reproduce and those haven't been provided going to close out for now. If still an issue though and issue mentioned in #5 doesn't address the problem please reopen.

If we remove the warning I’m fine with that. But pretty stuck in the default being true

longwave → credited smustgrave → .

So if we change to true are we fine to commit?

Definitely should have test coverage, but general approach seems fine. Though kinda would like to land ✨ New option "Wrap only text-like links." Active first since it seems to cover more usages.

smustgrave → created an issue.

Added missing threads and credit

Added missing threads and credit.