- Issue created by @bkosborne
- πΊπΈUnited States bkosborne New Jersey, USA
Okay, MR added and ready for review.
- πͺπΈ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.
- πͺπΈSpain fjgarlin
Small error in "phpcs". Once fixed, it can go directly into RTBC.
- πΊπΈUnited States bkosborne New Jersey, USA
Fixed! Once this is merged, I can start work on the other two issues I created.
-
fjgarlin β
committed 0d01c793 on 2.x authored by
bkosborne β
Issue #3532103 by bkosborne, fjgarlin: Modernize codebase & add tests
-
fjgarlin β
committed 0d01c793 on 2.x authored by
bkosborne β
- πͺπΈ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.