Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRM-21093: Call ->initialize() on the CiviCRM service now that Drupal 8 module has changed #11379

Merged
merged 1 commit into from
Dec 6, 2017

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Dec 6, 2017

While working on something in CRM_Util_System_Drupal8, I noticed that the ::loadBootstrap() method was getting the 'civicrm' service as a means to initialized CiviCRM, however, in civicrm/civicrm-drupal#495 we moved actual initialization to the initialize() method. The joy of circular dependencies! :-)

This PR simply adds the ->initialize() call. I searched for other instances that were missing this in civicrm-core, but couldn't find any


@colemanw colemanw merged commit f51dcc1 into civicrm:master Dec 6, 2017
@colemanw
Copy link
Member

colemanw commented Dec 6, 2017

@dsnopek I've merged this into master but unless we stick it in the 4.7.28-rc branch today it won't go into the 4.7.28 release. Is it a critical bug? FYI @totten

@dsnopek
Copy link
Contributor Author

dsnopek commented Dec 6, 2017

I haven't found anything that actually breaks on it, but I might just not be testing the things that caused that line to be added to that function. All the stuff we're testing will already have initialized CiviCRM before getting to that line anyway. I just happened to noticed that it was wrong when working on #11381

@colemanw
Copy link
Member

colemanw commented Dec 6, 2017

Ok sounds like it's good as-is.

sluc23 pushed a commit to ixiam/civicrm-core that referenced this pull request Jan 10, 2018
CRM-21093: Call ->initialize() on the CiviCRM service now that Drupal 8 module has changed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants