Follow-up from #2113659: Routing YAML for devel_generate .

changed module_exists to Drupal::moduleHandler()->moduleExists as per https://drupal.org/node/1894902

CommentFileSizeAuthor
#9 2116375-9-module_exists.patch9.57 KBjoelpittet
#4 2116375-4-module-exists-static-method.patch9.39 KBjoelpittet
#1 2116375-module_exists-replace.patch5.84 KBjoelpittet
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Active » Needs review
FileSize
2116375-module_exists-replace.patch5.84 KB

Here's the patch.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs review » Postponed

postponed and will need a re-roll after this gets in:
#2113659: Routing YAML for devel_generate

pcambra’s picture

pcambra
he/him
Primary language Spanish
Location Asturies
CreditAttribution: pcambra commented
Status: Postponed » Active
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Title: devel_generate replace module_exists with \Drupal::moduleHandler()->moduleExists » Replace module_exists with \Drupal::moduleHandler()->moduleExists
Component: devel_generate » devel
Status: Active » Needs review
FileSize
2116375-4-module-exists-static-method.patch9.39 KB

Might as well do this in one shot as there isn't much in other components.

pcambra’s picture

pcambra
he/him
Primary language Spanish
Location Asturies
CreditAttribution: pcambra commented

For some reason, tests are stuck again :\

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

Stuck how?

Status: Needs review » Needs work

The last submitted patch, 1: 2116375-module_exists-replace.patch, failed testing.

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented
Issue summary: View changes
Status: Needs work » Needs review

I reset the repo tests and they're green again.

Stuck how?

Before patch tests can run, the version in the repository must pass, i.e. https://drupal.org/node/3236/qa must be green. Whenever we push something, the testbot runs the repo test for the corresponding branch, and if the test fails, then patch testing is suspended.

I'm not sure whether repo testing continues automatically or not. IAC, it happens occasionally that d.o doesn't catch the answer from qa.d.o, and the repo tests won't turn green anymore. In that situation you have to delete and re-request them, which I did a couple of hour ago. After that, patch testing should resume.

However, #4 seems to be stuck on its own. But when you click the [View] link, you see that it failed.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
FileSize
2116375-9-module_exists.patch9.57 KB

Thanks for the detailed notes. Still a bit fuzzy on it but I see that it's green but viewing the test failed to apply patch.

So here's a re-roll.

pcambra’s picture

pcambra
he/him
Primary language Spanish
Location Asturies
CreditAttribution: pcambra commented 
+++ b/devel_generate/devel_generate.routing.yml
@@ -24,7 +24,7 @@ devel_generate.taxonomy_term:
-    _module_exists: 'taxonomy'

@@ -33,6 +33,6 @@ devel_generate.vocabulary:
-    _module_exists: 'taxonomy'

Not in the routing files please :)

pcambra’s picture

pcambra
he/him
Primary language Spanish
Location Asturies
CreditAttribution: pcambra commented
Status: Needs review » Needs work
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

@pcambra I was told that's how it is done... when you wrap the menu items wouldn't you want to also stop the routes from being created without that module as well? Maybe there is another way?

pcambra’s picture

pcambra
he/him
Primary language Spanish
Location Asturies
CreditAttribution: pcambra commented 
+++ b/devel_generate/devel_generate.routing.yml
@@ -24,7 +24,7 @@ devel_generate.taxonomy_term:
-    _module_exists: 'taxonomy'
+    _\Drupal::moduleHandler()->moduleExists: 'taxonomy'

@@ -33,6 +33,6 @@ devel_generate.vocabulary:
-    _module_exists: 'taxonomy'
+    _\Drupal::moduleHandler()->moduleExists: 'taxonomy'

Is this the way to do it? Looks a really odd thing to put in a yaml file

amateescu’s picture

amateescu CreditAttribution: amateescu commented

From what I see in https://drupal.org/node/1800686#requirements, _module_exists in routing files has to be _module_dependencies.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

@amateescu that does look right @pcambra mind if I change it to that?

_module_dependencies

pcambra’s picture

pcambra
he/him
Primary language Spanish
Location Asturies
CreditAttribution: pcambra commented
Status: Needs work » Fixed

After some changes I pushed this in, thanks @joelpittet, @amateescu

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.