Skip to content
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

(REF) Remove unused functions, setTemplateMenuValues() and getNavigation() #15274

Merged
merged 2 commits into from
Sep 11, 2019

Conversation

totten
Copy link
Member

@totten totten commented Sep 10, 2019

Overview

Remove unused functions.

These popped on my radar while grepping for consumers of userFrameworkURLVar (which needs to be refactored per dev/drupal#52). This just removes some noise from consideration.

Before

There are at least two unnecessary functions.

After

There are two less unnecessary functions.

Comments

More detailed comments are in the git commit notes.

Note that this is a `private` function, so any hypothetical callers must be
in the same class. There don't appear to be any.

Could there be some dynamic method calls?  Well, you'd expect there to be
some tell-tale such as references to `call_*()` or to a string which builds
up that method name from smaller strings ("set" or "MenuValues" or similar).
Nothing comes up.
I stumbled across this function while grepping to cleanup up
`userFrameworkURLVar` (dev/drupal#52).  Notice the first line of the
function throws a fatal error.  That line has been there since at least
CiviCRM v4.3.

https://github.com/civicrm/civicrm-core/blob/4.3/CRM/Core/Menu.php#L310

Unsurprisingly, if you grep for `getNavigation`, there are no calls to this
function.
@civibot
Copy link

civibot bot commented Sep 10, 2019

(Standard links)

@seamuslee001
Copy link
Contributor

Makes sense to me Merge on pass

@eileenmcnaughton eileenmcnaughton merged commit ae794e1 into civicrm:master Sep 11, 2019
@totten totten deleted the master-deadcode branch September 11, 2019 04:23
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.

3 participants