Meta issue: #2205673: [meta] Remove all @deprecated functions marked "remove before 8.0"

The drupal_is_front_page function is deprecated, so let's remove the usage of this function.

The removal of drupal_is_front_page is a seperate issue: #2388629: remove drupal_is_front_page().

Files: 
CommentFileSizeAuthor
#32 2388627-32.patch6.47 KBJeroenT
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,725 pass(es). View
#32 interdiff.txt1.54 KBJeroenT
#28 2388627-28.patch6.44 KBJeroenT
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,074 pass(es). View
#20 2388627-20.patch6.33 KBfilijonka
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,992 pass(es). View
#17 removed-deprecated-function-2388627-17.patch5.73 KBadci_contributor
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,695 pass(es), 144 fail(s), and 0 exception(s). View
#15 2388627-15.patch5.68 KBrpayanm
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,656 pass(es), 170 fail(s), and 17 exception(s). View

Comments

filijonka’s picture

Status: Needs review » Needs work

well you didn't actually remove the function itself and hence the file path.inc

JeroenT’s picture

Status: Needs work » Needs review

This issue is about removing the usages of drupal_is_front_page(). Removing the function itself is a seperate issue: #2388629: remove drupal_is_front_page().

filijonka’s picture

Issue summary: View changes

ok great, in the future it's always good if you add any related issues in the summary.

filijonka’s picture

Issue summary: View changes

Have you created an issue taking care of LinkGeneratorTest?

(sorry fat fingers so accidently added the q in summary

dawehner’s picture

Given how small that patch is I think we could also work on the LinkGeneratorTest bit here.

Otherwise this patch looks great, if green.

JeroenT’s picture

Issue summary: View changes
Issue tags: +Novice, +Needs reroll
Related issues: +#2388629: remove drupal_is_front_page().

@filijonka, my apologies. I added #2388629: remove drupal_is_front_page(). as a related issue.

There is no seperate issue for LinkGeneratorTest so we can do it here.

Also the patch no longer applies.

dawehner’s picture

Status: Needs review » Needs work
Issue tags: -Needs reroll

Alright, so let's be honest here.

tadityar’s picture

Status: Needs work » Needs review
FileSize
6.08 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,570 pass(es). View

Re-rolled the patch

tadityar’s picture

Status: Needs review » Needs work

Sorry didn't see the '-' sign on Needs re-roll

Status: Needs work » Needs review
ultimateAnkit’s picture

Status: Needs review » Needs work
Issue tags: +#dcdelhi

Sorry ,it was by mistake

ultimateAnkit’s picture

Status: Needs work » Needs review
FileSize
4.24 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,569 pass(es). View

this is my patch, please review.

Sivaji’s picture

Patch applied cleanly and no occurrences of call to drupal_is_front_page(). Good to mark as RTBC.

filijonka’s picture

Status: Needs review » Needs work

Hi

I got some questions about this reroll, in core/modules/Language/src/Plugin/Block/LanguageBlock.php the patch made by @JeroenT added some code, this is now gone. Why?

Stated in comment 4,5 and 6 we chould also take care of the call to drupal_is_front_page in LinkGeneratorTest, this is not done.

rpayanm’s picture

Status: Needs work » Needs review
FileSize
5.68 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,656 pass(es), 170 fail(s), and 17 exception(s). View

rerolled

Status: Needs review » Needs work

The last submitted patch, 15: 2388627-15.patch, failed testing.

adci_contributor’s picture

Status: Needs work » Needs review
FileSize
5.73 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,695 pass(es), 144 fail(s), and 0 exception(s). View

Trying to fix a fatal error of the patch in #15.
I guess it will help with exceptions.

Status: Needs review » Needs work

The last submitted patch, 17: removed-deprecated-function-2388627-17.patch, failed testing.

filijonka’s picture

Assigned: Unassigned » filijonka
Issue tags: -#dcdelhi

well something has gone really wrong here in #15 so done a reroll from first patch and will also fix LinkGenerator bf uploading.

filijonka’s picture

Status: Needs work » Needs review
FileSize
6.33 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,992 pass(es). View

sorry for this but it was easier for me to base this on first patch and do a reroll and removed the drupal_is_front_page in linkgeneratortest than trying to figure out what was wrong with later patches.

filijonka’s picture

Assigned: filijonka » Unassigned

Uccio queued 20: 2388627-20.patch for re-testing.

Uccio’s picture

Status: Needs review » Reviewed & tested by the community

The patch remove the use of drupal_is_front_page() and drupal works well.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +SprintWeekend2015
   * @todo Test that the active class is added on the front page when generating
   *   links to the front page when drupal_is_front_page() is converted to a
   *   service.

in \Drupal\Tests\Core\Utility\LinkGeneratorTest - this should be converted to a @todo pointing to a new issue about adding tests that active class is added on the front page.

segi’s picture

Assigned: Unassigned » segi
segi’s picture

Assigned: segi » Unassigned
filijonka’s picture

Assigned: Unassigned » filijonka

thanks @alexpott, was unsure how to deal with that and was uncertain if it still was a valid todo.

JeroenT’s picture

Assigned: filijonka » Unassigned
Status: Needs work » Needs review
FileSize
6.44 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,074 pass(es). View

Status: Needs review » Needs work

The last submitted patch, 28: 2388627-28.patch, failed testing.

Status: Needs work » Needs review

JeroenT queued 28: 2388627-28.patch for re-testing.

dawehner’s picture

+++ b/core/modules/language/src/Plugin/Block/LanguageBlock.php
@@ -34,6 +35,13 @@ class LanguageBlock extends BlockBase implements ContainerFactoryPluginInterface
+   * @var \Drupal\Core\Path\PathMatcher

@@ -44,10 +52,13 @@ class LanguageBlock extends BlockBase implements ContainerFactoryPluginInterface
+   * @param \Drupal\Core\Path\PathMatcher $path_matcher
...
+  public function __construct(array $configuration, $plugin_id, $plugin_definition, LanguageManagerInterface $language_manager, PathMatcher $path_matcher) {

Note: We have a PathMatcherInterface, so let's use it.

JeroenT’s picture

FileSize
1.54 KB
6.47 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,725 pass(es). View

@dawehner, you're right. Thank you for your review!

New patch attached.

JeroenT’s picture

Issue summary: View changes
Canutza’s picture

Patch in #32 applied cleanly, also checked for drupal_is_front_page() and no occurrences of it.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Great, I think this is done then.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 32: 2388627-32.patch, failed testing.

Canutza’s picture

This doesn't seem right, possible because of this #2417549: Drupal\migrate_drupal\Tests\d6\MigrateFileTest fail in MigrateTestBase
I guess the patch is ok.

Status: Needs work » Needs review

cilefen queued 32: 2388627-32.patch for re-testing.

JeroenT’s picture

Status: Needs review » Reviewed & tested by the community

RTBC as said in #35.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 7c0e0e0 and pushed to 8.0.x. Thanks!

The beta evaluation is in the meta issue.

  • alexpott committed 7c0e0e0 on 8.0.x
    Issue #2388627 by JeroenT, filijonka, ultimateAnkit, tadityar, rpayanm,...

Status: Fixed » Closed (fixed)

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