Modernize codebase & add tests

Created on 24 June 2025, 18 days ago

Problem/Motivation

We can modernize the codebase a bit now that Drupal and PHP have evolved. E.g., NodeSelectTerms.php can use constructor property promotion, return types, typed arguments, etc. We also need some automated tests.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

πŸ“Œ Task
Status

Active

Version

1.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States bkosborne New Jersey, USA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @bkosborne
  • Merge request !8Modernize codebase and add tests. β†’ (Merged) created by bkosborne
  • πŸ‡ΊπŸ‡ΈUnited States bkosborne New Jersey, USA

    Okay, MR added and ready for review.

  • Pipeline finished with Success
    17 days ago
    Total: 197s
    #531352
  • Pipeline finished with Canceled
    16 days ago
    Total: 54s
    #531392
  • Pipeline finished with Success
    16 days ago
    Total: 240s
    #531396
  • Pipeline finished with Success
    16 days ago
    Total: 169s
    #531433
  • Pipeline finished with Success
    16 days ago
    Total: 142s
    #531447
  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    Thanks for this. I will review it shortly. If this goes through, I’m thinking of creating a 2.x branch as we’re dropping support for older versions and adding support for newer ones.

    I haven’t checked the code in detail yet, but if we require a minimum version of PHP to leverage the new language features we should add that requirement to the composer file too.

  • πŸ‡ΊπŸ‡ΈUnited States bkosborne New Jersey, USA

    Yea, I think we're good at just requiring Drupal 10, which requires PHP 8.1 already. I don't think I used any language features beyond 8.1.

  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    I just created the 2.x branch and moved the target branch of the MR to be that. I left some feedback on the MR, really minimal, and answer the question in the `@todo`. Feel free to do something about it here or in a follow-up.

    The code looks really good and it'd be ready to merge into a 2.x branch.

    I'll let you see the feedback first before marking it RTBC. Great job!

  • πŸ‡ΊπŸ‡ΈUnited States bkosborne New Jersey, USA

    Thanks for the review! I addressed the feedback and pushed.

  • Pipeline finished with Failed
    15 days ago
    Total: 166s
    #533136
  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    Small error in "phpcs". Once fixed, it can go directly into RTBC.

  • Pipeline finished with Success
    15 days ago
    Total: 255s
    #533218
  • πŸ‡ΊπŸ‡ΈUnited States bkosborne New Jersey, USA

    Fixed! Once this is merged, I can start work on the other two issues I created.

  • Pipeline finished with Skipped
    15 days ago
    #533258
  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    The MR is merged now. Thanks a lot for the improvements so far! Looking forward to reviewing more issues.

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

Production build 0.71.5 2024