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

Remove calls to deprecated conf_path() #1607

Closed
iant-ee opened this issue Sep 10, 2015 · 14 comments
Closed

Remove calls to deprecated conf_path() #1607

iant-ee opened this issue Sep 10, 2015 · 14 comments

Comments

@iant-ee
Copy link

iant-ee commented Sep 10, 2015

conf_path() is due to be removed before Drupal 8.0.0 (see https://www.drupal.org/node/2457469 ), but doing so breaks drush. We need to remove calls to this function.

@junowilderness
Copy link
Contributor

It seems like if many usages of conf_path were changed to drush_conf_path, and drush_conf_path made aware of anything it needs to know for 8, that may work.

@weitzman
Copy link
Member

Yeah, I suggest that drush_conf_path() be added to the three commands/core/drupal/environment*.inc files. Then when you use drush_conf_path(), you should add this line right above it - drush_include_engine('drupal', 'environment');

@iant-ee
Copy link
Author

iant-ee commented Sep 16, 2015

How are you getting on cliefen? Is this ready for review?

@junowilderness
Copy link
Contributor

@iant-tui See #1611

@iant-ee
Copy link
Author

iant-ee commented Sep 22, 2015

Can I help with this in any way? It is a blocker to conf_path() being removed, and if we don't do that before RC1 (announced as 7th Oct) we'll need to support it until Drupal 9.

@junowilderness
Copy link
Contributor

@iant-tui, in #1611, @weitzman suggested re-implementing conf_path in the bootstrap classes. You could try that.

@weitzman
Copy link
Member

Sorry I got delayed. You can help by making a PR.

@iant-ee
Copy link
Author

iant-ee commented Sep 22, 2015

I take it you haven't written any code for this yet then. Are we basically suggesting copying D8's current conf_path() function into the bootstrap class?

@weitzman
Copy link
Member

Correct I have not started. I have not investigated the best way to implement this. There may be a better way than just copying the code - I dont yet know.

@junowilderness
Copy link
Contributor

@weitzman Can you give me a hint on a way to test the changes, like a command that will definitely error or fail if the changes don't work?

@weitzman
Copy link
Member

Full bootstrap would fail if this code broke so I think we have sufficient implicit testing already.

@weitzman
Copy link
Member

Coded up a fix at #1607. Will merge when tests are green.

@junowilderness
Copy link
Contributor

The PR is #1631

@paul-m
Copy link

paul-m commented Sep 26, 2015

Actually removing conf_path() from Drupal is blocked by this issue.

https://www.drupal.org/node/2573443

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants