Add change record to @deprecated for install_ensure_config_directory() in install.inc.

CommentFileSizeAuthor
#2 add_change_record_to-2873750-2.patch545 byteseheller

Comments

eheller created an issue. See original summary.

eheller’s picture

StatusFileSize
new545 bytes
eheller’s picture

Assigned: eheller » Unassigned
eheller’s picture

Status: Active » Needs review
sorabh.v6’s picture

Status: Needs review » Reviewed & tested by the community

Hey @eheller, @see have been added to install_ensure_config_directory. I confirm that no other @deprecated without @see is left in install.inc

I'm setting to RTBC. Thanks All

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: add_change_record_to-2873750-2.patch, failed testing. View results

dinesh18’s picture

#patch 2 doesn't seems to follow depreciation policy.

mile23’s picture

#7: This issue is very limited in scope and is attempting to work on only some parts of deprecation.

+++ b/core/includes/install.inc
@@ -530,6 +530,8 @@ function drupal_install_config_directories() {
+ * @see https://www.drupal.org/node/2501187

git log -L520,540:core/includes/install.inc reveals that the @deprecation was added in #2466197: Staging directory should not have to be writeable., which does not have a change record.

So we should add #2466197: Staging directory should not have to be writeable. to the list of related issues for change record https://www.drupal.org/node/2501187

Restarting the test of #2 since it was never tested.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

angel.h’s picture

Added the issue to the change record and since the test on #2 passed I'm moving this to "Needs review".

angel.h’s picture

Status: Needs work » Needs review
angel.h’s picture

Status: Needs review » Reviewed & tested by the community

Looks fine -> RTBC.

janec’s picture

I'm at drupalcon Vienna and will try to add a @see for the deprecated install_ensure_config_directory() function
oeps. It's done already. Sorry.

mile23’s picture

Added 8.5.x test to #2, to make sure it all works.

xjm’s picture

Title: Add Change record to @deprecated for install.inc » Add Change record to @deprecated for install_ensure_config_directory()
angel.h’s picture

Test agains 8.5.x passed as well.

alexpott’s picture

According to https://www.drupal.org/core/deprecation we should also be adding an @trigger_error() to install_ensure_config_directory() - but we can't just do that because this method is used in \Drupal\simpletest\KernelTestBase::prepareConfigDirectories(). We need a follow-up issue to complete the deprecation of this method by removing the usages (just inlining the functionality into KernelTestBase::prepareConfigDirectories()) and to add the @trigger_error() call.

alexpott’s picture

catch’s picture

Version: 8.5.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

  • catch committed 942b70b on 8.5.x
    Issue #2873750 by eheller: Add Change record to @deprecated for...

  • catch committed 207ba89 on 8.4.x
    Issue #2873750 by eheller: Add Change record to @deprecated for...
alexpott’s picture

Status: Fixed » Closed (fixed)

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

kay_v’s picture

Removing parent issue per conversation with @xjm at Drupalcon Nashville Mentored Sprint prep. Her recommendation to do so was based on a few points that made sense to all of us in the discussion, namely:
- so many child issues makes this parent unwieldy
- search filters will allow people needing to refind closed children