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

Simplify class inheritance #23121

Merged
merged 1 commit into from
Apr 7, 2022
Merged

Simplify class inheritance #23121

merged 1 commit into from
Apr 7, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Apr 6, 2022

Overview

Simplify class inheritance

Before

class CRM_Contribute_Import_Parser_Contribution extends CRM_Contribute_Import_Parser {

After

Functions from CRM_Contribute_Import_Parser moved to CRM_Contribute_Import_Parser_Contribution so everything is on the same class.

Technical Details

CRM_Contribute_Import_Parser_Contribution is the only class in our universe to
extend CRM_Contribute_Import_Parser and this class only adds confusion to the
mix as functions are 'distributed' between them

image

image

Comments

@civibot civibot bot added the master label Apr 6, 2022
@civibot
Copy link

civibot bot commented Apr 6, 2022

(Standard links)

@eileenmcnaughton eileenmcnaughton force-pushed the imp branch 2 times, most recently from 82a9eb0 to 0fe4ebe Compare April 6, 2022 23:14
CRM_Contribute_Import_Parser_Contribution is the only class in our universe to
extend CRM_Contribute_Import_Parser and this class only adds confusion to the
mix as functions are 'distributed' between them. This removes the inheritance,
leaving only the constant that is still used in the now-deprecated class
@colemanw
Copy link
Member

colemanw commented Apr 7, 2022

Simplification makes sense. There's no real purpose to using an abstract class just once, it might as well not exist.

@colemanw colemanw merged commit 3c6fa26 into civicrm:master Apr 7, 2022
@colemanw colemanw deleted the imp branch April 7, 2022 01:47
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.

2 participants