Problem/Motivation

Remaining tasks

  1. The change record for this issue should include a link to recommendations page, https://www.drupal.org/node/3223395#s-olivero. (For example, the CR for removing HAL)
  2. Tag this issue 'Needs release note.'
  3. Remove the extension ;-).
  4. Update update path tests as needed. See Learn how to write an automated update test.
  5. Remove references from core/phpstan-baseline.neon.
  6. Remove any spelling words specific to the extension from the dictionaries.
  7. Add the extension to the relevant removed list, either DRUPAL_CORE_REMOVED_MODULE_LIST or DRUPAL_CORE_REMOVED_THEME_LIST, in system.install.
  8. Check for references in @todo.

Release notes snippet

The EXTENSION_NAME {module/theme}has been removed from core, and can now be installed as a contrib {module/theme}.

CommentFileSizeAuthor
#14 3595089-nr-bot_r2hwmqyq.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3595089

Command icon 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:

About issue forks

Comments

quietone created an issue. See original summary.

quietone opened merge request !16727

gábor hojtsy made their first commit to this issue’s fork.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

#3622600: Remove remaining uses of Olivero in tests just landed in main. Updated the MR, running tests now. Fingers crossed, it should be green. I think this will still be blocked on deprecating Olivero itself though if green?

quietone’s picture

quietone commented

There are uses of Olivero in the Search module, which should have been removed by now. That is held up on the creation of the contrib project.

quietone’s picture

quietone commented

And it is still in the database dumps. Is there an existing issue for that I wonder.

catch’s picture

catch
he/him
Primary language English
commented
Related issues: +#3623927: Remove Olivero from database dumps

I think we can remove Olivero with the references from search module still in search, since we know we're going to remove that too. Or whichever direction can be done without causing test failures - either way it's OK if contrib search/olivero integrate.

Opened #3623927: Remove Olivero from database dumps for removing Olivero from the database dumps, I don't think we already had an issue for that, was missed in the last round of database dump updates.

quietone’s picture

quietone commented

Added 'cpsell:ignore olivero' in the Search module, so cspell passes now. And local testing suggests that when #3623927: Remove Olivero from database dumps is committed, all tests will pass here.

catch’s picture

catch
he/him
Primary language English
commented
Status: Postponed » Reviewed & tested by the community

Committed the database dump MR, apart from cspell changes, this is purely the removal now, and everything looks good.

I like the Olivero theme a lot but things have changed dramatically with Drupal CMS and the slimmed down standard profile over the past couple of years.

quietone’s picture

quietone commented

The Nightwatch tests in Search rely on olivero_test. So those tests are failing.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs review

In #3586217-7: Remove the Search module the last search failures for removal are Olivero related. Do we need to combine the issues to remove both at the same time?

quietone’s picture

quietone commented

In Slack @gábor hojtsy pointed out that the contrib Search module already exists and the nightwatch tests are passing there. So, it is safe to remove these tests here.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Removing just those tests is fine and a much better idea than combining the removal issues which would be harder to follow in git history etc. Moving back to a hopefully more solid RTBC again.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3595089-nr-bot_r2hwmqyq.txt91 bytes

The 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.

larowlan made their first commit to this issue’s fork.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs work » Reviewed & tested by the community

#3621198: Change remaining usages of Claro touched core/themes/olivero/templates/filter/filter-tips.html.twig

  • larowlan committed 8d87e802 on main 
    task: #3595089 Remove the Olivero theme

By: quietone
By: gábor hojtsy...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to main - end of an era.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

quietone’s picture

quietone commented

Step 7 was missed here. The extension was not added to DRUPAL_CORE_REMOVED_THEME_LIST, in SystemRequirements

quietone opened merge request !17186

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Fixed » Needs review
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Reviewed & tested by the community

New MR looks good to me

  • xjm committed 205375d4 on main 
    task: #3595089 (followup) Remove the Olivero theme

By: quietone
By:...
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

I committed the followup to main. I had to do so with --no-verify because cspell was failing checking the spelling of maintainers' names in MAINTAINERS.txt, which should be ignored. My hypothesis is that the recent CSpell update broke something but tests on the above MR passed, so 🤷‍♀️.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.