Problem/Motivation

Now that #3338356: Shift requirements to Drupal 10 has landed, we can adopt new PHP 8.1 features and remove a lot of boilerplate:

14 files changed, 84 insertions(+), 516 deletions(-)

See #3321474: Adopt PHP 8.1-only capabilities such as constructor property promotion + drop BC layers for inspiration

Steps to reproduce

Proposed resolution

Remaining tasks

  • ✅ File an issue about this project
  • ☐ Manual Testing
  • ☐ Code Review
  • ☐ Accessibility Review
  • ☐ Automated tests needed/written?

Issue fork project_browser-3342128

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett created an issue. See original summary.

tim.plunkett opened merge request !359

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
CreditAttribution: chrisfromredfin at Redfin Solutions, LLC commented

Would love it if someone wanted to rebase this and continue the work. Everyone loves to delete code!

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

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
CreditAttribution: elber at CI&T commented
Status: Active » Needs work

I just rebased it.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
CreditAttribution: chrisfromredfin at Redfin Solutions, LLC commented
Status: Needs work » Needs review

@tim is this everywhere that we can do this, or are there other locations? I'll test it manually if we think it's good to go.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented

Previous rebase was more of a merge, and didn't pass tests. Manually rebased using `git rebase` and pushed.

@chrisfromredfin this should be all of it

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
CreditAttribution: chrisfromredfin at Redfin Solutions, LLC commented
Status: Needs review » Reviewed & tested by the community

OK, I have tested this thoroughly manually in DrupalPod, and I reviewed the code diff.

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

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented
Status: Reviewed & tested by the community » Needs work

Left a comment in the MR with something that seems to be missing. Other than that one, the rest of the code looks good.

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented
Status: Needs work » Fixed

Thanks for the quick fix, I rechecked the code again and retested everything via drupalpod (including drush commands) and everything was good. As the issue was already RTBC and the feedback was really minor and fixed right away, I went ahead and merged it.

Status: Fixed » Closed (fixed)

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