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

Change function buildEventFeeForm to non-static #16337

Merged
merged 1 commit into from
Jan 22, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Change function buildEventFeeForm to non-static

Before

public static function buildEventFeeForm(&$form) {

After

public function buildEventFeeForm($form) {

Technical Details

I searched on thee full function name & also 'buildEvent' & 'feeForm' and I don't believe this is called
from outside this function. I want to call it from tests to fix some test set up issues butt
there are a bunch of undefined properties so I concluded I should set this up first.

Also passing form by reference is unnecessary

Comments

I searched on thee full function name & also 'buildEvent' & 'feeForm' and I don't believe this is called
from outside this function. I want  to call it from tests to  fix some test set up issues butt
there are a bunch of undefined properties so I concluded I should set this up first.
@civibot
Copy link

civibot bot commented Jan 19, 2020

(Standard links)

@civibot civibot bot added the master label Jan 19, 2020
@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

test this please

@demeritcowboy
Copy link
Contributor

  • General standards
    • [r-explain] PASS
    • [r-user] PASS
    • [r-doc] PASS
    • [r-run] PASS
      • The messages seem to all work from front and backoffice. Also checked waitlist messages/functionality.
  • Developer standards
    • [r-tech] PASS
      • Agree it doesn't seem to be called from elsewhere.
    • [r-code] PASS with comment
      • A next logical step would be to remove the parameter completely and replace $form with $this in the function, since passing $this to $this doesn't add anything.
    • [r-maint] PASS (Description suggests there's a test coming.)
    • [r-test] PASS

@seamuslee001
Copy link
Contributor

agree with @demeritcowboy 's review here merging

@seamuslee001 seamuslee001 merged commit 6baf465 into civicrm:master Jan 22, 2020
@seamuslee001 seamuslee001 deleted the ev_static branch January 22, 2020 00:23
@eileenmcnaughton
Copy link
Contributor Author

Thanks @demeritcowboy & @seamuslee001 & yes - totally agree about removing $form

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.

3 participants