Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Over in #2232605: Themes cannot be uninstalled @catch pointed out that array_diff(array_keys($foo), array_keys($bar));
is the same as array_keys(array_diff_key($foo, $bar));
. We should convert all the instances of this pattern in core
Comment | File | Size | Author |
---|---|---|---|
#11 | ConfigImporter-2340701-11.patch | 5.75 KB | martin107 |
#8 | ConfigImporter-2340701-8.patch | 5.66 KB | rosinegrean |
#5 | ConfigImporter-2340701-5.patch | 5.63 KB | rosinegrean |
#1 | ConfigImporter_2340701.patch | 2.06 KB | rosinegrean |
Comments
Comment #1
rosinegrean CreditAttribution: rosinegrean commentedThe rest of the class seems ok for me.
Comment #2
martin107 CreditAttribution: martin107 commented@prics, Hello...
What you have done looks correct ....
I read the issue slightly differently
array_keys(array_diff_keys($foo, $bar))
by searching the whole repository for
'array_diff(array_keys'
I see that the bad patterns has been used 10 times throughout core.... including the 4 that your have already caught.
Comment #3
alexpottI've scanned the ConfigImporter and can't find anything else obvious to clean up so how about re-scoping to fix all
array_diff(array_keys(), array_keys())
in core. Seems sensible to me.Nice patch @prics.
btw...
in EntityReferenceIntegrationTest can not be converted.
Comment #4
alexpottComment #5
rosinegrean CreditAttribution: rosinegrean commentedUpdated the patch.
Comment #6
martin107 CreditAttribution: martin107 commentedI applied the patch and can confirm there are no more instances of the old way of doing this
Visually the patch look good to me... does exactly what is required and no more.
With the scope of the issue so specific ... I am happy to RTBC it.
Comment #8
rosinegrean CreditAttribution: rosinegrean commentedComment #11
martin107 CreditAttribution: martin107 commentedSimple fixup in field.api.php
hook_field_storage_config_update_forbid()
Comment #12
rosinegrean CreditAttribution: rosinegrean commentedComment #13
KingdutchPatch applied cleanly and the following regular expression ran on the entire project yielded no results:
array_diff(\s*array_keys\(.{2,}\),\s*array_keys\(.{2,}\)\s*)
The pattern should find the following:
array_diff(
followed by none or any whitespace. Thenarray_keys(
with a variable in there. This done twice separated by a comma and any possible whitespace with the closing)
Patch quality looks like that of a well done find and replace. This all together with the patch passing all existing tests makes me mark this RTBC.
Comment #16
KingdutchI must've been blind, I thought this was all green, my apologies for marking this RTBC.
However.. the error seems to be in an unrelated part of Drupal so I re-queued the test. If I'm wrong we'll find out soon enough and we will have to do some digging.
Comment #17
rosinegrean CreditAttribution: rosinegrean commentedNo problem, the tests passed before RTBC. I've runned in my local the tests a few min ago and they all pass.
Comment #18
omessaoudi CreditAttribution: omessaoudi commentedI will test this
==> Patch ran without any problem fo me too
Comment #19
omessaoudi CreditAttribution: omessaoudi commentedComment #20
omessaoudi CreditAttribution: omessaoudi commentedComment #23
skipyT CreditAttribution: skipyT commentedthanks martin107 and prics, looks ok!
Comment #24
webchickSounds like catch wanted this.
Comment #25
catchCommitted/pushed to 8.0.x, thanks!