Problem/Motivation
Remove deprecated code from Utility component
\Drupal\Component\Utility\Crypt::randomBytes\Drupal\Component\Utility\Crypt::hashEquals\Drupal\Component\Utility\SafeMarkup\Drupal\Component\Utility\Unicode::getStatus\Drupal\Component\Utility\Unicode::strlen\Drupal\Component\Utility\Unicode::strtoupper\Drupal\Component\Utility\Unicode::strtolower\Drupal\Component\Utility\Unicode::substr\Drupal\Component\Utility\Unicode::caseFlip\Drupal\Component\Utility\Unicode::strpos
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | 3091447-19.patch | 31.59 KB | krzysztof domański |
| #19 | interdiff-16-19.txt | 841 bytes | krzysztof domański |
| #16 | 3091447-16.patch | 30.77 KB | krzysztof domański |
| #16 | interdiff-14-16.txt | 620 bytes | krzysztof domański |
| #14 | 3091447-14.patch | 30.37 KB | krzysztof domański |
Comments
Comment #2
el7cosmosComment #3
el7cosmosComment #4
el7cosmosComment #5
el7cosmosComment #6
el7cosmosComment #7
martin107 commented@el7cosmos thanks for talking the time to sort all this out...
1) I have visually scanned this patch and all the changes make sense to me.
2) When I search for @trigger or @deprecated in the Utilities directory
All looks clear, the hits in DeprecatedArray should remain.
3) The patch applies
4) The are no additional coding standard errors.
Comment #8
mile23Patch in #5 needs a reroll.
Comment #9
ravi.shankar commentedI have re-rolled.
Comment #10
martin107 commentedComment #11
martin107 commentedComment #12
longwave#9 only covered the Unicode class, this adds Crypt and SafeMarkup.
Comment #13
berdirNeeds a reroll, the includes removal removed one method as well I think.
Comment #14
krzysztof domańskiRe-rolled. #3062757: Remove deprecated legacy include files from Drupal 9 was commited.
Comment #15
krzysztof domańskiUnused class TextWrapper after we delete SafeMarkupTest. However not marked as deprecated. Can we also remove it?
core/tests/Drupal/Tests/Component/Utility/TextWrapper.phpComment #16
krzysztof domańskiComment #17
longwaveRe #15 I think that is fine to delete, it was only ever intended for use with the companion class.,
Comment #18
kristen polPer a Slack discussion with Gábor Hojtsy regarding usage of D9 tags (Drupal 9, Drupal 9 compatibility, Drupal 9 readiness, etc.), "Drupal 9 compatibility" should be used for contributed projects that need updating and "Drupal 9" was the old tag for D8 issues before the D9 branch was ready. Doing tag cleanup here based on that discussion.
Comment #19
krzysztof domańskiAddressed #17.
Comment #20
berdirLooking at existing "SafeMarkup" references in core..
We have \Drupal\KernelTests\Component\Utility\SafeMarkupKernelTest for example. We could say that's in for the word "safe markup", but at least the class docblock has an explicit "SafeMarkup" reference. And the only thing it really seems to test is :url-style placeholders. Not sure if renaming a test and test methods is something we want to do, so maybe just the docblock?
Then we have SafeMarkupTestMarkup, a test clas in TwigMarkupInterfaceTest, but there's not really a reference to SafeMarkup, so OK?
Comment #21
longwaveRaised #3098244: Rename SafeMarkupKernelTest to FormattableMarkupKernelTest to handle the first part. I think that the SafeMarkupTestMarkup test class is OK, it is nothing to do with the SafeMarkup concept.
Comment #22
berdirLooks good to me, we have a separate issue for that one badly named test class.
Comment #24
catchCommitted 3447368 and pushed to 9.0.x. Thanks!