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

dev/core#562 remove calls to dao->free() from api folder #13393

Merged
merged 1 commit into from
Jan 7, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Code clean up with no functional change

Before

no change

After

no change

Technical Details

Per gitlab

The DAO object since ? 4.7.x? has been freed on _destruct. Using the ->free action has been demonstrated to create some rare bugs - ie. because query sets from the outer loop can be lost. There is no benefit in calling it any more and some harm

#13192 was the last one in this series

Comments

@aydun

Per gitlab

The DAO object since ? 4.7.x? has been freed on _destruct. Using the ->free action has been demonstrated to create some rare bugs - ie. because query sets from the outer loop can be lost. There is no benefit in calling it any more and some harm

civicrm#13192 was the last one in this series
@civibot
Copy link

civibot bot commented Jan 3, 2019

(Standard links)

@civibot civibot bot added the master label Jan 3, 2019
@eileenmcnaughton
Copy link
Contributor Author

test this please

@seamuslee001
Copy link
Contributor

Jenkins re test this please

@seamuslee001
Copy link
Contributor

This is consistent with other cleanup work. Only real check is Jenkins confirming nothing breaks which in this case Jenkins suggests it is ok.

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 did you mean to merge this?

@seamuslee001
Copy link
Contributor

ah yes

@seamuslee001
Copy link
Contributor

merging

@seamuslee001 seamuslee001 merged commit 46703d6 into civicrm:master Jan 7, 2019
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