Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dietrich Schmidt created an issue. See original summary.

dietric@gmail.com’s picture

dietric@gmail.com’s picture

Status: Active » Needs review
FileSize
579 bytes

Here's the patch for this

johnshortess’s picture

Status: Needs review » Needs work

I'm seeing extra whitespace at the end of the two added comment lines. Other than that, it looks like the patch is linking to the correct change records, and the format of the comment matches the format from the parent issue.

I believe @Dietrich Schmidt has already left the sprint -- can anyone else reroll this patch to fix the whitespace issue?

kwhite’s picture

I was working on other deprecation tags in this file. Can I roll all of them together? If so happy to take this on...

johnshortess’s picture

@kwhite I think that would be fine. Just please be sure to add @Dietrich Schmidt on your issue so he gets credit for his patch! Thanks!

kwhite’s picture

Will do!

kwhite’s picture

Wait.... how do I do that @johnshortess??? Issue is here: https://www.drupal.org/node/2873796

kwhite’s picture

Made the whitespace adjustments and also added change records for the other deprecated flags in the file. Patch attached.

kwhite’s picture

Status: Needs work » Needs review
davidneedham’s picture

This is very close! Everything looks good to me except for the next to last change - lines

+++ b/core/includes/bootstrap.inc
@@ -742,6 +757,8 @@ function drupal_installation_attempted() {
+ *
+ * @see https://www.drupal.org/node/2235431

This (https://www.drupal.org/node/2235431) seems to be an old change record that isn't specific enough to this particular function. It uses drupal_get_profile() in the example (which doesn't quite seem appropriate if it's depreciated) and doesn't mention that the function itself is being depreciated.

I think https://www.drupal.org/node/2538996 is closer. Part-way down it says: "To access the install profile in Drupal 8 use \Drupal::installProfile() or inject the install_profile container parameter into your service." (which matches the comment) as well as "drupal_get_profile() has been deprecated and will be removed in Drupal 9."

I rerolled the patch with this change. Leaving the status on "Needs Review".

cosmicdreams’s picture

Title: Add Change record to @deprecated for format_string in bootstrap.inc » Add Change record to @deprecated in bootstrap.inc

Modifying title to reflect current scope of issue.

cosmicdreams’s picture

Status: Needs review » Reviewed & tested by the community

I reviewed the previous patch and David's change. Looks good to me.

catch’s picture

Status: Reviewed & tested by the community » Needs work

Thanks for this!

+++ b/core/includes/bootstrap.inc
@@ -953,6 +970,7 @@ function drupal_static_reset($name = NULL) {
  *
  * @see \Drupal\Component\Utility\SafeMarkup::format()
+ * @see https://www.drupal.org/node/2302363
  */

We should remove the reference to SafeMarkup::format() here, since SafeMarkup itself has been deprecated.

+++ b/core/includes/bootstrap.inc
@@ -315,6 +325,9 @@ function t($string, array $args = [], array $options = []) {
  * @deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0.
  *   Use \Drupal\Component\Render\FormattableMarkup.
+ *
+ * @see https://www.drupal.org/node/2302363
+ * @see https://www.drupal.org/node/1312890
  */
 function format_string($string, array $args) {
   return SafeMarkup::format($string, $args);

Let's just add the reference to 2302363, the older change record is made obsolete by that one.

pritish.kumar’s picture

Did the changes as specified in #14. Please check if it's correct.

gaurav.kapoor’s picture

Status: Needs work » Needs review
catch’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! Patch looks good now so moving back to RTBC.

  • Gábor Hojtsy committed ad4b958 on 8.3.x
    Issue #2873749 by pritish.kumar, kwhite, dietric@gmail.com, davidneedham...
Gábor Hojtsy’s picture

Title: Add Change record to @deprecated in bootstrap.inc » Add change record links to @deprecated items in bootstrap.inc
Version: 8.4.x-dev » 8.3.x-dev
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

  • Gábor Hojtsy committed e7c6471 on 8.4.x
    Issue #2873749 by pritish.kumar, kwhite, dietric@gmail.com, davidneedham...

Status: Fixed » Closed (fixed)

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

xjm’s picture

Removing the issue relationship from fixed issues to keep the parent issue usable. Thanks!