Problem/Motivation
In #3215045: [Duplicate] Highlight obsolete module's status at admin/reports/status page, providing warning and link with explanation we discovered that there were 2 themes who used experimental: true instead of lifecycle: experimental in their .info.yml. We're changing that in issue #3250585: Highlight deprecated modules and themes at admin/reports/status page, providing warning and link with explanation.
However unlikely: There could be Contrib out there who also use experimental: true therefore we need a nice deprecation for this.
Exactly for that this issue was opened.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3250342
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:
- 3250342-deprecate-experimental-true
changes, plain diff MR !2955
Comments
Comment #2
spokjeThis issue is postponed on #3250585: Highlight deprecated modules and themes at admin/reports/status page, providing warning and link with explanation.
Comment #3
spokjeComment #4
spokjeComment #5
spokjeComment #6
spokjeComment #7
quietone commentedTurns out there are uses of 'experimental: true' in contrib.
Comment #8
spokjeThanks @quietone
We'll file issues in those modules queues when we start this issue I suppose?
Comment #9
spokjeUnpostponing since #3250585: Highlight deprecated modules and themes at admin/reports/status page, providing warning and link with explanation is committed.
Comment #13
spokjeComment #14
spokjeComment #16
smustgrave commentedRebased as the MR was unmergable it said.
All the tests appear to be green so think this good.
Comment #17
catchFeels bad asking (and too late to reduce any work) but do we really need test coverage for this one?
Comment #18
spokjeAlways unsure about test coverage, so my default is to add it.
If we can do without, I'm happy to remove it from the MR.
Comment #19
longwaveGiven that we are only adding to one method, maybe it only needs a small unit test that covers the deprecation?
Also, is there a better place to do this in
InfoParserDynamic::parse()?Comment #20
quietone commentedChange is failing tests. :-(
Comment #21
spokjeComment #22
spokjeLet's see if this works as minimal testing
Comment #23
smustgrave commentedSee minimal fixed the test cases.
@catch I imagine the "Legacy experimental test" check is for a test theme so it doesn't run that code? That's what I saw at least.
Comment #24
spokjeAnswered the question posted by @catch in https://www.drupal.org/project/drupal/issues/3250342#mr2955-note135516
Comment #25
xjmThis is ready overall, but let's remove that out-of-scope change with the typehint. Thanks!
Comment #27
rpayanmComment #28
xjmThanks @rpayanm! That's exactly the fix we need.
Comment #29
xjmSince I was only RTBCing a very small change, I'm comfortable committing this myself even though I re-RTBCed it.
Committed to 10.1.x, and published the change record. Thanks everyone!