Problem/Motivation

Since statistics is being deprecated make sure there are no references to the module outside of the module itself.

Steps to reproduce

Proposed resolution

See #3340834-7: [meta] Tasks to deprecate Statistics for a grep command and list of links. Hopefully that is all of the ones to check but finding the instances of 'statistics' to change or remove is not so easy.

Remaining tasks

Review.

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 report.txt15.33 KBsmustgrave

Issue fork drupal-3410210

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

quietone created an issue. See original summary.

sourabhjain’s picture

sourabhjain
Primary language English
commented

Let me work on this.

sourabhjain opened merge request !5931

quietone’s picture

quietone commented
Status: Active » Needs review

@sourabhjain, when you work on an issue it is good practice to comment on what you did and what still remains to be done. By the MR I can see that you did the first item in the 'remaining tasks' but did you do the second step?

I am going to set this to needs review. I have not searched for other instances.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
StatusFileSize
new report.txt15.33 KB

Do not believe a search was done.

Did a phpstorm search and manually reviewed the findings

Y2038TimestampUpdateTest as reference to statistics module that could be removed

But think it would be good to postpone this on moving statistics tests as a lot of comments seems to be attached to tests, example in commentStatistics.

Ignored findings in Forum

quietone’s picture

quietone commented

This the reference in Y2038TimestampUpdateTest

      // Start with a standard install of Drupal 9.3.0 with the following
      // enabled modules: forum, language, locale, statistics and tracker.
      DRUPAL_ROOT . '/core/modules/system/tests/fixtures/update/drupal-9.4.0.filled.standard.php.gz',
      DRUPAL_ROOT . '/core/modules/system/tests/fixtures/update/Y2038-timestamp.php',

The reference to statistics there can stay because it actually is installed in drupal-9.4.0.filled.standard.php.gz.

And the changes to htaccess breaks tests.

quietone’s picture

quietone commented
Status: Needs work » Needs review

Looking for how to remove statistics and have the tests pass

quietone’s picture

quietone commented
Issue summary: View changes

Removed the work on the htaccess and tests to a sibling issue.

quietone’s picture

quietone commented
Issue summary: View changes

Ready for review

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs review » Needs work

All changes look sensible, did a search, most references that came up are for fixtures and migrations, htaccess is a separate issue.

I believe this is the last straggler that we overlooked?
We caught it in the actual autoload.php, just not in the generator of the file: https://git.drupalcode.org/project/drupal/-/blob/11.x/composer/Plugin/Sc...

EDIT: This an edge case of a comment being written in code, so this might be already taken care of in another sibling issue?

quietone’s picture

quietone commented
Status: Needs work » Needs review

@Spokje, Thank you for finding a straggler!

spokje’s picture

spokje
Primary language Dutch
commented

Thanks @quietone, I would RTBC this, but I see @sourabh_jain_419 left a comment on the MR.

Not sure why composer/Plugin/Scaffold/GenerateAutoloadReferenceFile.php shouldn't be changed?

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs review » Reviewed & tested by the community

Added an explanation in the MR why I think it _should_ be changed.

Marking as RTBC to let core committers decide on it.

larowlan made their first commit to this issue’s fork.

larowlan credited catch.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Committed to 11.x, thanks

larowlan closed merge request !5931

  • larowlan committed 4a530812 on 11.x 
    Issue #3410210 by quietone, sourabhjain, smustgrave, Spokje, catch:...

Status: Fixed » Closed (fixed)

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