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] duplicate function. #17400

Merged
merged 1 commit into from
May 27, 2020
Merged

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

This seems like the opposite of good principles but I've found that when facing a nasty chunk of code it's
often better to copy it & then start to clean it up, knowing that other places are not affected & not
having as much complexity to deal with

Before

Function shares

After

Membership recur form to 'got it alone' - no cleanup or change as yet

Technical Details

Comments

@civibot
Copy link

civibot bot commented May 26, 2020

(Standard links)

@civibot civibot bot added the master label May 26, 2020
@mattwire
Copy link
Contributor

@eileenmcnaughton checkstyle.. Also what is your plan here? This is a pretty big function that is called a lot! I'm not sure I see the benefit vs. cleaning it up in place. This function usually gets hit whenever I'm looking into membership issues too.

@eileenmcnaughton eileenmcnaughton force-pushed the recurr branch 3 times, most recently from df2f312 to 426ace4 Compare May 26, 2020 11:02
@eileenmcnaughton
Copy link
Contributor Author

@mattwire it's just too hard to clean up in place - there is too much going on & this is the simple use case whereas the front end form usage is complex.

About 50% of the function can probably be ripped out in the first round as I believe this line

https://github.com/civicrm/civicrm-core/pull/17400/files#diff-349ca8bcf1ef264bf3ca9acc2593aa54R768

should always be true.

In essence what the function does is mostly come up with an array to pass to Membership::create so it's just a case of untangling that

@mattwire
Copy link
Contributor

@eileenmcnaughton Test failures relate

This seems like the opposite of good principles but I've found that when facing a nasty chunk of code it's
often better to copy it & then start to clean it up, knowing that other places are not affected & not
having as much complexity to deal with
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