Problem/Motivation

  • update_process_project_info
  • update_calculate_project_data
  • update_calculate_project_update_status

Steps to reproduce

Proposed resolution

Create UpdateCalculate service
public projectData
protected projectInfo
protected projectUpdateStatus

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3580705

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

nicxvan created an issue. See original summary.

nicxvan’s picture

nicxvan commented
Issue summary: View changes
dww’s picture

dww
we/he/they
commented
Related issues: +#3100110: Convert update_calculate_project_update_status() into a class

Not duplicate, but definitely related to #3100110: Convert update_calculate_project_update_status() into a class. I haven’t had time or urgency to circle back to that issue. I sort of remember not fully agreeing with where it ended up, but there’s a lot of prior work in there that needs to either be refreshed and finished or at least considered before proceeding here.

My initial concern with this plan (and its siblings) is we already have some weird services that are sort of incomplete and interact with each other in “interesting” ways. I wasn't around core for a few years when folks started OOP-ifying this module. I don't love any of how it’s currently split up. I'd love to take some time (which I don't have in the next ~3 weeks) to really think about it and come up with a more wholistic plan for how we should organize all this. I'd rather do that before we start introducing new services.

nicxvan’s picture

nicxvan commented

We will do our best not to make this worse, but we do want to deprecate and move these functions.

nicxvan’s picture

nicxvan commented

Didn't do deprecations for new parameters, not sure how to handle a final and private constructor either.

I also didn't inject key value expirable, still need to do that.

I didn't update the CR yet.

nicxvan opened merge request !15524

nicxvan’s picture

nicxvan commented

For the private final classes I just added the new parameter.
I added the deprecation.

I also added a custom CR for this that needs updating.

nicxvan’s picture

nicxvan commented
Status: Active » Needs review

This is ready for review.

nicxvan’s picture

nicxvan commented
Status: Needs review » Needs work

Strict types is causing a couple of failures, I can't review why right now though.

nicxvan’s picture

nicxvan commented
Status: Needs work » Needs review

I reached out to @godotislate in slack for help and here helped be find the cause in the build tests.

It turns out it's actually pretty deep in the extension so not something to fix here.

@godotislate said it would be OK to drop strict types and leave a comment.

Edit: definitely need to give @godotislate credit, but I'm on my phone and not logged into new.d.o

nicxvan’s picture

nicxvan commented
Status: Needs review » Needs work

Got a few things to address, thanks!

nicxvan’s picture

nicxvan commented

Ok I addressed most of the feedback, we still need to introduce the value object for project information.