Skip to content

(dev/core#174) api_v3_SettingTest - Remove dead code #12321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 16, 2018

Conversation

totten
Copy link
Member

@totten totten commented Jun 15, 2018

Overview

This unit test is clearing out a non-existent cache item. It's dead-code.

Before

  • SettingTest clears CiviCRM setting Spec, which is never populated anyway

After

  • CiviCRM setting Spec has disappeared from the face of the Earth.

Technical Details

To see that it the cache item is non-existent, I grepped (case-insensitive) for the expressions:

  • CiviCRM setting Spec
  • CiviCRM setting
  • setting spec

This feels like left-overs from 4.6. In 4.7, the setting cache was reworked. Note that SettingTest does clear the newer cache (Civi::cache('settings')->flush();).

This unit is clearing out a non-existent cache item. To see that it is non-existent, I grepped (case-insensitive) for
the expressions:

* `CiviCRM setting Spec`
* `CiviCRM setting `
* `setting spec`

This feels like left-overs from 4.6. In 4.7, the setting cache was reworked. Note that
`SettingTest` does clear newer cache (`Civi::cache('settings')->flush();`).
@civibot
Copy link

civibot bot commented Jun 15, 2018

(Standard links)

@seamuslee001
Copy link
Contributor

Merging as per tag

@seamuslee001 seamuslee001 merged commit e783f87 into civicrm:master Jun 16, 2018
@totten totten deleted the master-unused-setitem branch June 16, 2018 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants