Problem/Motivation
At #3290808-20: Remove workspaces special casing from system_requirements() and fix versions/docs I pointed out:
+++ b/core/modules/system/system.install
@@ -1368,18 +1368,16 @@ function (callable $hook, string $module) use (&$module_list, $update_registry,
+ '@current_major' => 10,
Probably out of scope here, but why hard-code this instead of getting it from \Drupal::VERSION?
Steps to reproduce
Proposed resolution
Use \Drupal::VERSION not hard-coded integers to print out the current major version of core.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 12340.patch | 1.26 KB | niranjan_panem |
| #5 | diff_reroll_3292490_2-4.txt | 1.88 KB | ankithashetty |
| #5 | 3292490-4.patch | 1012 bytes | ankithashetty |
| #2 | 3292490-2.patch | 1012 bytes | immaculatexavier |
| #2 | 3292490-2.patch | 1012 bytes | immaculatexavier |
Issue fork drupal-3292490
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
Comment #2
immaculatexavier commentedI've attached the patch in accordance with the proposed resolution.
Comment #3
spokjeSadly the patch doesn't apply. It looks like it was against an outdated version of
10.0.x-dev.Comment #4
dwwThanks for moving this along. Also need work since \Drupal::VERSION is the full version (eg 10.0.1). We need to parse just the major version out of that.
Comment #5
ankithashettyRerolled the patch and addressed #4, thanks!
Comment #6
longwavePatch looks good to me.
This issue reminded me of #3134417: Add \Drupal::getMajorVersion() as well.
Comment #7
dwwI know I said “current” in the title, but if we’re doing this much, what about
'@previous_major'2 lines down?Comment #8
longwaveIf we are to do that we should move
@required_min_versionto a constant somewhere and then calculate@previous_majorfrom that I think?Comment #9
dwwYeah, perhaps. I'm not sure the best way to handle this, given that part of it *does* have to be hard-coded by a human. You're probably right that a constant is better than what we have here. It seems like we should find all these hard-coded version strings peppered throughout core and unify their handling so we don't have to track them down each time we rev a major version.
This issue reminded me of another issue, too. I struggled to locate it at first, but then I found #3110223: Add ModuleVersion::getMinorVersion() method. I see that's totally different from #3134417: Add \Drupal::getMajorVersion(), so definitely nothing duplicate. Thanks for the link, following #3134417 now...
Comment #10
smustgrave commentedThis issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 200
Moving to NW for an agreed upon solution. Though sounds like a single instance is the approach.
Thanks
Comment #12
mstrelan commentedConsidering
\Drupal::VERSIONis currently11.3-devand we have10hardcoded I guess 10 is wrong? That would make this a bug.Comment #15
niranjan_panem commentedCreated patch for Drupal Version 11. https://www.drupal.org/files/issues/2025-06-10/12340.patch