Use a consistent name for the aggregator module throughout core

Naming issues go to documentation component.

Module UI text goes into the module-specific component.

The attached patch changes every occurrence of "feed aggregator" to "aggregator".

#3: change-feed-aggregator-to-aggregator-202783.patch queued for re-testing.

Changed the title form Feed aggregator to Aggregator d8 core module.

The tag does not include a #.

Patch applied and no issues. We at the Toronto Drupal User Group on #SprintWeekend have agreed that it's a good idea to drop it off too since we have it only in three places and it's all menus. It's a better idea to keep it consistent through out Drupal.

Hm. Passing to the UX team for feedback. While in general I favour fewer words, not sure if it's as clear what we're aggregating without "Feed" in front.

Thank you @webchick. it is good idea to pass to UX team.

reroll

Removing sprint weekend tag!!
As suggested by @YesCT

@deepakaryan1988 The tag was added January 17, 2015 during the actual global sprint weekend. See https://groups.drupal.org/node/447258
Please leave the tag on issues that were actually worked on during that event.

I think you might have confused a message from May about the sprint weekend tag, that it should not be added to events from May. The tag is for issues worked on during https://groups.drupal.org/node/447258 by people attending sprint weekend sprints.

If this issue is still relevant, then we should take it up during the sprints at DrupalCon Barcelona before we are at RC1.

That patch needs to be rerolled and the related tests needs to be fixed.
Both are doable by a novice.

Updating title to something more descriptive and not potentially confusing with vocabulary terms.
Updating IS to use template.

Re-rolling the patch from https://www.drupal.org/node/202783#comment-9886399. Conflict due to new lines added to $output and !xxx placeholder replaced with :xxx.

Bleh, re-rolling https://www.drupal.org/node/202783#comment-10370663 with a test fix.

Patch 202783-32 works as expected, updates the text to aggregator.

Checking patch applied to Acquia dev desktop (8.0.x):
Several places showing up changed successfully:
Home Administration Configuration
Home Administration Configuration Web Services
Home Administration Configuration Web Services Feed aggregator
Home Feed aggregator Sources
Home

However, not appearing in Extend, and it is flipped in the blocks section. (Instead of Feed aggregator, it is Aggregator feed) (see images)

in this file /core/modules/migrate_drupal/tests/fixtures/drupal6.php there are also instances of "Feed aggregator" should we also change those?

I think dinaizida is right, otherwise a site converted from D6 would have the wrong title on Aggregator pages, right?

Tested successfully in simplytest.me, as in screen captures above, all instances I could find of "Feed aggregator" in the UI now say "Aggregator".

Made change to D6 migration based on suggestion by dinaizida in #38.

The patch seems to apply and catches most of the uses of Feed aggregator.

However I found one more in migrate.

grep -rl 'Feed aggregator'

core/modules/migrate_drupal/tests/fixtures/drupal7.php

I found 6 instances that will need to be patched.

Replacement patch that takes into account apratt's suggestion to also change the migrate_drupal tests code for Drupal 7. Only updated 4 instances, not 6, to be consistent with the Drupal 6 tests code changes made in the earlier patch. (The other two instances aren't titles, so left them unchanged.)

Patch looks good to me. It would be great if someone from UX team give some insights.

Patch looks good to me too.

Above issue verified and screenshot attached here.

When a branch moves from beta to rc (which happened already in 8.1.x) string changes are not allowed.
And 8.2.x is in beta (and 8.3.x is open for development) https://groups.drupal.org/node/512705

Looks like string changes might be allowed in the 8.2.x beta phase
https://www.drupal.org/core/d8-allowed-changes#beta

@chetan2111 thanks for reviewing and uploading and embedding those screen shots.

This issue still has some "remaining tasks" in the issue summary, and the proposed resolution seems to need to be updated also. Setting to needs work, so that a committer that looks at an rtbc issue will find the correct information. :)

Also, this is not minor, but looks like normal according to https://www.drupal.org/core/issue-priority#normal

I'm not sure if the feedback since @webchick asked for a usability review has been done (See #12). Since then, we have some more process in place https://www.drupal.org/governance/core (adding needs usability review tag to get clarity on that)

Updated issue summary.

We discussed this at our weekly UX meeting. Lets go for "Aggregator". It's shorter and you will learn about feeds as the input for it by using it.

Back to RTBC then I guess. Thanks @YesCT for the triage!

I wanted to double check all the instances were removed since #43 is from January, the code might have changed.

Good news, the patch applies cleanly to 8.2.x and 8.3.x

with patch applied in 8.2.x, and 8.3.x:

ag -i "Feed aggregator" core
core/modules/aggregator/src/Tests/FeedAdminDisplayTest.php
6: * Tests the display of a feed on the feed aggregator list page.

core/modules/migrate_drupal/tests/fixtures/drupal7.php
18307:  'options' => 'a:1:{s:10:"attributes";a:1:{s:5:"title";s:129:"Configure the behavior of the feed aggregator, including when to discard feed items and how to present feed items and categories.";}}',
22030:  'description' => 'Configure the behavior of the feed aggregator, including when to discard feed items and how to present feed items and categories.',

core/modules/migrate_drupal/tests/fixtures/drupal6.php
14935:  'source' => 'Feed aggregator',

Next I'm gonna look into those more and see if they are relevant to this change. Maybe not, since in d6 and d7 its name isn't (yet) changed.

[edit: berdir pointed out in irc when I asked, that the drupal7.php and drupal6.php are dumps. :) so those should not be changed. Looking into FeedAdminDisplayTest.php next]

I looked at

core/modules/aggregator/src/Tests/FeedAdminDisplayTest.php
6: * Tests the display of a feed on the feed aggregator list page.

Looked at the test, and it is referring to admin/config/services/aggregator
I installed drupal, enabled aggregator module and looked at that page.
And it is the "List" tab on the Aggregator page, so changing this makes sense to me here.

This is in a comment in a test, so it's not a huge deal either way.

This is the only comment change in the patch now. I am surprised this was the only instance, but it also underlines the point of this patch pretty strongly :) Thanks for verifying.

:)

adding some info to the summary that will hopefully help (committers) review it.

Committed/pushed to 8.3.x and cherry-picked to 8.2.x. Thanks!

+++ b/core/modules/aggregator/src/Tests/FeedAdminDisplayTest.php
index 347b2f2..2e35c31 100644
--- a/core/modules/migrate_drupal/tests/fixtures/drupal6.php

--- a/core/modules/migrate_drupal/tests/fixtures/drupal6.php
+++ b/core/modules/migrate_drupal/tests/fixtures/drupal6.php

+++ b/core/modules/migrate_drupal/tests/fixtures/drupal6.php
index bbc8088..db919a6 100644
--- a/core/modules/migrate_drupal/tests/fixtures/drupal7.php

--- a/core/modules/migrate_drupal/tests/fixtures/drupal7.php
+++ b/core/modules/migrate_drupal/tests/fixtures/drupal7.php

These are test fixtures with database dumps of d6 and d7 sites so we should make a follow up to revert this section of the patch.

@neclimdul: wups, wanna RTBC #2792133: Quickfix: Undo inadvertent aggregator test fixture changes then? :)