Problem/Motivation
See parent(s).
All the simple conversions are for #3402292: Fix strict type errors: Convert FormattableMarkup to strings (simple replacement) in core/tests/Drupal/KernelTests/* and #3402293: Fix strict type errors: Convert FormattableMarkup to strings (simple replacement) in core/modules/*/tests/src/Kernel/*. This issue is for all the more complicated places where we need rewrite how the message is generated. If that's still too huge a set of changes, we can split this one up futher.
Steps to reproduce
Proposed resolution
Convert "everything else" from FormattableMarkup into strings (e.g. with new local variables, other simplifications, etc).
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | 3402294-nr-bot.txt | 90 bytes | needs-review-queue-bot |
Issue fork drupal-3402294
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
mstrelan commentedSplit these changes from the parent issue, marking NW to make readability improvements.
Comment #5
mstrelan commentedAddressed feedback
Comment #6
smustgrave commentedAll feedback/threads appear to be addressed.
Comment #7
mstrelan commentedMR needs rebasing
Comment #8
mstrelan commentedRebased, seems like some of the changes were already addressed in a related issue.
Comment #9
smustgrave commentedRebase seems good.
Comment #10
quietone commentedI found a stray character and left a question about using
...$args.Comment #11
mstrelan commentedFixed the stray % character. Perhaps we should get rid of the $args array in those sprintf calls and just add the values directly?
Comment #12
smustgrave commentedRemoving that sounds like it could be a follow up as that could lead down a rabbit hole (I imagine)
Comment #13
quietone commentedI don't this it is a rabbit hole for reasons I mention in the MR. I wanted to use the suggestion feature for these two changes but it wasn't working for me \Drupal\Tests\system\Kernel\Form\ProgrammaticTest::doSubmitForm so I abandoned that idea.
Comment #14
mstrelan commentedDo we even need a $message here? Would make all these issues so much easier. Anyway, that's a topic for another day.
Comment #15
quietone commentedI agree, most of these don't need a message.
To get this moving again I made the changes I suggested.
Comment #16
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #17
quietone commentedArg, I've had trouble with git with this issue. Should be sorted now.
Comment #18
smustgrave commentedAppears feedback has been addressed
Comment #19
mstrelan commentedThank you @quietone, have looked over the last 3 commits and am happy with these changes.
Comment #20
alexpottCommitted and pushed 955418c2df to 11.x and c9255990e0 to 10.3.x. Thanks!
Committed ac127f0 and pushed to 10.2.x. Thanks!
Backported to 10.2.x to keep tests aligned...
We should file a followup to change \Drupal\Tests\file\Kernel\FileManagedUnitTestBase::assertFileHooksCalled() to something like:
The
$this->assertTrue(FALSEis bizarre... but not added here.