Closed (fixed)
Project:
Drupal core
Version:
11.x-dev
Component:
postgresql db driver
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
24 Apr 2025 at 03:19 UTC
Updated:
23 Jul 2025 at 14:04 UTC
Jump to comment: Most recent
Comments
Comment #2
dwwCopying some relevant comments from the MR threads...
dww
nicxvan
dww
Comment #3
nicxvan commentedPackageManagerRequirements has some minor duplication as well.
Comment #4
dwwRe #3: isn’t that #3520888: Review duplicated requirements check in package manager?
Comment #5
nicxvan commentedyes, thank you!
Comment #6
dwwLet’s be more specific so we don’t confuse ourselves. 😅
Comment #7
larowlanThe blocker is in, I think a static method is the best approach if possible
Comment #8
dww@larowlan: What's wrong with my plan to just instantiate the install-time object in the runtime OOP method and call it directly? Why does it have to be moved into a
staticmethod?We might have other chances to establish the "state of the art" in this space over at #3493718: Convert system_requirements() into OOP hooks and install time class, but this is a much simpler initial case to tackle. 😅 Personally, I think it's cleaner to instantiate the install object and use it at runtime than it is to refactor both to rely on a
staticmethod. Whatever DI hurdles / restrictions the install time object must conform to means it's a subset of what would be allowed at runtime, so I don't see how it could break.Thoughts?
Thanks,
-Derek
Comment #9
dwwp.s. Are we sure this is "Novice" material? This stuff is pretty thorny, with some tricky bits to get right. Not sure we want this to be someone's "first contribution to core", etc...
Comment #10
larowlan@dww not fussy either way, if we can cleanly instantiate it - works for me. Removing the Novice tag
Comment #11
dwwCool, thanks.
More accurate title. 😉
Comment #12
dwwOh hah, I forgot that
InstallRequirementsInterface::getRequirements()is alreadypublic static. 😂Working on an MR, stay tuned.
Comment #14
dwwPretty simple, other than #2909480: Move REQUIREMENT_* constants out of install.inc file. Pipeline is green on both MySQL and PgSQL.
Comment #15
dwwRebased after #3410938: Create enums for RequirementSeverity and deprecate drupal_requirements_severity() constants landed. Removed the yucky now that we've got an enum instead of needing to include install.inc.
Comment #16
smustgrave commentedSeems pretty straight forward. The same code is in PgsqlRequirements so this seems like a clean replacement.
Comment #18
longwaveCommitted 1459e06 and pushed to 11.x. Thanks!