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

Move processConfirm function from Utils file back to form class #19212

Merged
merged 1 commit into from
Dec 14, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Dec 14, 2020

Overview

Move processConfirm function from Utils file back to only form still using it

Before

Large processing function only called by the confirm form, but located on a different class

After

Function moved to the Confirm the form class

Technical Details

This function is really part of the Confirm form. People thought sharing it would
be good once. They were wrong.

More specifically sharing large blocks of
code that attempt to service many diferent needs is not helpful

Comments

…using it

This function is really part of the Confirm form. People thought sharing it would
be good once. They were wrong. More specifically sharing large blocks of
code that attempt to service many diferent needs is not helpful
@civibot
Copy link

civibot bot commented Dec 14, 2020

(Standard links)

@civibot civibot bot added the master label Dec 14, 2020
@eileenmcnaughton eileenmcnaughton changed the title Move processConfirm function from Utils file back to only form still … Move processConfirm function from Utils file back to form class Dec 14, 2020
@seamuslee001
Copy link
Contributor

Test coverage would show us if there is some issue here merging

@seamuslee001 seamuslee001 merged commit 9d4d416 into civicrm:master Dec 14, 2020
@seamuslee001 seamuslee001 deleted the form_move2 branch December 14, 2020 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants