Problem/Motivation

The file system is pretty large and removing deprecations from the module involved moving from the lib folder so decided to break out.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3575575

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

smustgrave created an issue. See original summary.

smustgrave opened merge request !14899

smustgrave’s picture

smustgrave commented
Assigned: smustgrave » Unassigned
Status: Active » Needs review
dcam’s picture

dcam commented
Status: Needs review » Needs work

I left suggestions on the MR. There are a bunch of @throws \ValueError annotations for legacy integer parameters. I assume these should be removed since going forward PHP should throw a TypeError for them. But if I'm wrong for some reason, then you can reject the suggestions.

There are a few deprecations in https://git.drupalcode.org/project/drupal/-/blob/main/core/lib/Drupal/Co.... For the record, its legacy test was removed.

smustgrave’s picture

smustgrave commented
Status: Needs work » Needs review

Applied the suggestions directly to the MR. Thanks for finding the those |int I thought I got all of them.

dcam’s picture

dcam commented
Status: Needs review » Needs work

It looks like the deprecations in https://git.drupalcode.org/project/drupal/-/blob/main/core/lib/Drupal/Co... should be in the scope of this issue. Let me know if I'm wrong.

smustgrave’s picture

smustgrave commented
Status: Needs work » Needs review

Not sure how I missed that.

dcam’s picture

dcam commented
Status: Needs review » Reviewed & tested by the community

Looks good. Thank you.

My feedback has been addressed. This looks good to me.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Reviewed & tested by the community » Needs work

Left a few comments for additional removals.

smustgrave’s picture

smustgrave commented
Assigned: Unassigned » smustgrave

Will try and address in the morning!

And correct migrations are handled separately in https://www.drupal.org/project/drupal/issues/3572280

Thanks for the review!

smustgrave’s picture

smustgrave commented
Status: Needs work » Needs review

Feedback addressed. Thanks!

dcam’s picture

dcam commented
Status: Needs review » Needs work

IMO, we need to remove both the deprecation trait and property if we're removing either of them.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented

Agree we should remove stuff if it's no longer used.

smustgrave’s picture

smustgrave commented
Status: Needs work » Needs review

Addressed the feedback.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented

This is looking good. Ready to RTBC once we confirm the test fail was random.

dcam’s picture

dcam commented

That test passes for me locally, but for what it's worth the test isn't listed on the list of known, intermittent failures.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented

As a sub-system maintainer, I should be able to re-run the tests, but can't 🤔

dcam’s picture

dcam commented

Yeah, that ability broke a few days ago.

smustgrave’s picture

smustgrave commented

Rebased the MR to try and trigger a rebuild but think ConfigInstallProfileOverrideTest is random

dcam’s picture

dcam commented
Status: Needs review » Reviewed & tested by the community

Yay, it passed.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

That's a big chunk of code removed.

Committed/pushed to main, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed 3f8e552b on main 
    refactor: #3575575 Remove deprecations around File component and module...

Status: Fixed » Closed (fixed)

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