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

Unused functions in CRM_Upgrade_Form #24361

Merged
merged 1 commit into from
Aug 23, 2022

Conversation

demeritcowboy
Copy link
Contributor

Overview

They can't be used because they contain calls to nonexistent functions. It's easy to see:

cv ev "(new CRM_Upgrade_Form())->getTemplateFileName();"
Error: Call to undefined method CRM_Upgrade_Form::getTemplateMessage() in CRM_Upgrade_Form->getTemplateFileName() (line 238 of ...\CRM\Upgrade\Form.php).

cv ev "(new CRM_Upgrade_Form())->postProcess();"
Error: Call to undefined method CRM_Upgrade_Form::upgrade() in CRM_Upgrade_Form->postProcess() (line 244 of ...\CRM\Upgrade\Form.php).

@civibot
Copy link

civibot bot commented Aug 23, 2022

(Standard links)

@totten
Copy link
Member

totten commented Aug 23, 2022

But it's called upgrade form. There must be a template! And a submit button! 🙃

Anyway, nice catch. Agree these functions aren't usable.

I've wondered before about renaming CRM_Upgrade_Form to reflect the fact that it's not, well, a form. I was concerned about breaking external calls (eg universe/ext/*/CRM/*/Upgrader/Base.php), but we could probably leave a class_alias() behind.

@demeritcowboy
Copy link
Contributor Author

You could revert this commit: civicrm/civicrm-svn@4fc48c7

@eileenmcnaughton eileenmcnaughton merged commit f5eb717 into civicrm:master Aug 23, 2022
@demeritcowboy demeritcowboy deleted the unused-funcs branch August 23, 2022 23:43
@colemanw
Copy link
Member

colemanw commented Sep 1, 2022

Wow, CiviCRM was so much simpler back then. Imagine having such green pastures to work in today.

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