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

Minor comment fixes & function extraction. #9751

Merged
merged 3 commits into from
Mar 18, 2017

Conversation

eileenmcnaughton
Copy link
Contributor

No description provided.

@totten totten added the master label Feb 6, 2017
* Get all the phone numbers for a specified location_block id, with the primary phone being first
* Get all the phone numbers for a specified location_block id, with the primary phone being first.
*
* This is called from CRM_Core_BAO_Block as a calculated function.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is good form to say in comments where something is called from, as that can get dated.

@JoeMurray
Copy link
Contributor

The code changes look fine, but I haven't tested to see if the refactoring of some code into two functions is working, nor checked to see if $tree was being accessed from outside the class inappropriately and thus making it protected causing a error to be raised.

@seamuslee001
Copy link
Contributor

tested this and looks file to me

@seamuslee001
Copy link
Contributor

@eileenmcnaughton have rebased it for you

@eileenmcnaughton
Copy link
Contributor Author

added merge on based based on @seamuslee001 testing

@eileenmcnaughton eileenmcnaughton merged commit 6bb0014 into civicrm:master Mar 18, 2017
@eileenmcnaughton eileenmcnaughton deleted the report branch March 18, 2017 01:49
monishdeb pushed a commit to monishdeb/civicrm-core that referenced this pull request May 2, 2017
Minor comment fixes & function extraction.
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