Problem/Motivation
Steps to reproduce
Proposed resolution
Change comments to remove references to Olivero.
Uses not change here
core/modules/locale/tests/src/Functional/LocaleThemeInstallTest.php - do in a new issue concerning remaining tests
Search is about to be deprecated and removed, so leave these is Search
- core/modules/search/tests/src/Nightwatch/Tests/Olivero/oliveroRemoveBlockConsoleTest.js
- core/modules/search/tests/src/Nightwatch/Tests/Olivero/TestSiteOliveroInstallTestScript.php
core/tests/Drupal/Nightwatch/Tests/Claro/claroPrimaryTabsTest.js - A valid comment that states the file came from Olivero. This will leave core when Claro is removed.
Remaining tasks
Review
Commit
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|
Issue fork drupal-3617523
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 #3
quietone commentedComment #4
quietone commentedComment #5
quietone commentedLinting is passing an this is only changing comments, so setting for a review.
Comment #7
smustgrave commentedApplied the suggestions to use Default Admin over claro.
I did use AI to help with the grep command grep -rni "olivero" /var/www/html/core --include="*.php" --include="*.twig" --include="*.yml" | grep -v "/themes/olivero/" | grep -E "\s*(//|#|\*|{\s*#)"
This still gets findings but reviewing them it appears to be related to tests are being removed/moves in other tickets.
May need one final lap at the very end.
Comment #8
longwaveTiny grammar nitpick; I think "an SVG" is correct over "a SVG"!
Comment #9
quietone commented@longwave, thanks for the suggestions. I applied the suggestions.
Linting passed on this doc only MR so setting to needs review.
@smustgrave, not sure about that grep command. It is not searching the entire repository and is only searching a few file extensions. We do have comments in *.js and *.module etc. And there is a usage in a JSON file as well.
git grep -ni olivero | grep -v core/assets | grep -v core/themes/olivero | grep -E "\s*(//|#|\*|{\s*#)"Comment #10
smustgrave commentedRunning that I get
Spot checking a few I think they're related to tests. So may be good to do one more lap after all the tests land too.
Comment #11
quietone commentedRebased and postponing. I need to do some move searching but that will have to wait until tomorrow.
Comment #12
quietone commentedComment #13
quietone commentedThis is in better shape now. Linting has passed so setting to needs review.
Comment #14
gábor hojtsyOnly found one very minor nit. I think it looks good once that is fixed.
Comment #15
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #16
quietone commentedIt is tempting to change Claro here, but that is out of scope here.
Also rebased for recent commit
Comment #17
godotislate1 tiny grammar nit, ok to self RTBC.
Comment #18
quietone commentedApplied the suggestion. thanks
Comment #19
godotislatelgtm