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.
Abstract the code from block visibility for php filter check to be a condition plugin. Separate this from the existing current path check.
Eclipse
Parent Issue:
Comment | File | Size | Author |
---|---|---|---|
#23 | 1921546-23.patch | 4.66 KB | EclipseGc |
#17 | 1921546-17.patch | 5.47 KB | EclipseGc |
#17 | 1921546-interdiff.txt | 3.63 KB | EclipseGc |
#15 | 1921546-15.patch | 4.75 KB | EclipseGc |
#11 | condition-php-1921546.11.patch | 4.91 KB | larowlan |
Comments
Comment #1
larowlanComment #2
larowlanComment #4
larowlanFailed because testbot out of diskspace.
Comment #5
larowlan#2: condition-php-1921546.2.patch queued for re-testing.
Comment #7
larowlanStill failing because out of diskspace
Comment #8
podaroktag
Comment #9
larowlanRe-roll to reference container namespaces.
Comment #11
larowlanThese pass locally, but merging HEAD anyway
Comment #12
larowlanbump
Comment #13
EclipseGc CreditAttribution: EclipseGc commented#11: condition-php-1921546.11.patch queued for re-testing.
Comment #15
EclipseGc CreditAttribution: EclipseGc commentedOk, let's see how this one fairs.
Comment #16
tim.plunkettJust some nitpicks, this looks good.
Why not just
$this->manager = $this->container('plugin.manager.condition');
You're already using $this->container.
Should have a docblock, "Modules to enable." is the standard comment.
No docblocks needed here. Yeah.
s/php/PHP
assertTrue
Just a matter of style, but I think its normally
$this->assertEqual($variable, 'string');
, so just switching them.Comment #17
EclipseGc CreditAttribution: EclipseGc commentedAlrighty then! Since conditions aren't actively used by anything, apparently they were not updated for the namespace stuff so I fixed that in this patch.
Eclipse
Comment #18
tim.plunkettThe code and tests look good. Without a use case or UI, I can't judge it by any other criteria. So, RTBC.
Comment #19
webchickNo tests..?
Comment #20
tim.plunkettTest here!
Comment #21
webchickLOL sorry about that! Chrome froze loading mid-patch and so I only saw the first two hunks. :)
I will have some D8 time Wednesday so will try and get to it then, if not before. Alex/catch/etc. feel free to beat me to it. :D
Comment #22
alexpottNeeds a re-roll
Comment #23
EclipseGc CreditAttribution: EclipseGc commentedremoved the core bundle stuff since that patch went in.
Eclipse
Comment #24
tim.plunkettLooks great!
Comment #25
alexpottCommitted a015229 and pushed to 8.x. Thanks!
Comment #26
larowlanupdated change notice at http://drupal.org/node/1961370
Comment #27
alexpottForgot to mention... during the commit I updated the documentation to the latest standards...
All updated to {@inheritdoc} ... see http://drupalcode.org/project/drupal.git/commit/a015229