Problem/Motivation

Today, I told @tedbow that alarm bells are going off in my mind as I review the version validation code in Automatic Updates. It feels too complicated. Too many moving parts. The policy is hard to follow.

That reminds me of another thing: _update_calculate_project_status(). That code is a spaghetti-flavored eldritch horror and is barely maintainable. I think we are unwittingly heading down the same path.

Proposed resolution

We need to change direction before we find ourselves in a quagmire of impossible code from which we cannot escape. We need to simplify the way we encode the version policy in code.

I discussed with Ted and we think that what might be best is to have all version validation, both starting and target versions, attended and unattended, in one final class. We should split the logic into small, well-named, easy to follow methods. The class should be final because this is our policy we're talking about. We don't want people extending or overriding parts of it. Either you take our whole policy, or you reject it entirely.

Remaining tasks

Merge all version validators into a single long final class with many small, well-named, well-scoped, private methods.

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:

Comments

phenaproxima created an issue. See original summary.

phenaproxima’s picture

Assigned: Unassigned » tedbow
Status: Active » Needs review

Assigning to @tedbow for review.

tedbow’s picture

Status: Needs review » Needs work

for more things still . very close though

phenaproxima’s picture

Status: Needs work » Needs review
tedbow’s picture

Status: Needs review » Reviewed & tested by the community

@phenaproxima amazing work!

phenaproxima’s picture

I absolutely think @tedbow deserves credit for what he put into this. I may have written the code but this is absolutely the product of two brains.

  • phenaproxima committed d90ebb9 on 8.x-2.x
    Issue #3280180 by phenaproxima, tedbow: Consolidate all version...
phenaproxima’s picture

Status: Reviewed & tested by the community » Fixed

Merged into 8.x-2.x. Thanks!

Status: Fixed » Closed (fixed)

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