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

CRM-20226 [NFC] add comment to warn coders about performance impact #11011

Merged
merged 1 commit into from Sep 26, 2017

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Sep 21, 2017

Overview

Adds a code comment notifying devs about the risk of improving group resolution.

@monishdeb the line commented is a change you made. I've decided the performance issue is manageable for us after a group tidy up but can you read & merge for future.


@seamuslee001
Copy link
Contributor

@eileenmcnaughton does that generate the smart group count?

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 yes - some time back there was discussion about generating the count on-demand rather than always, for performance reasons.

The change I'm grumpy about brought back the 'always' partially - ie. it only applies to the first 25 groups (no matter how many are displayed)

@seamuslee001
Copy link
Contributor

jenkins re test this please

@monishdeb
Copy link
Member

Makes sense.

@monishdeb monishdeb merged commit f0bcec1 into civicrm:master Sep 26, 2017
@eileenmcnaughton eileenmcnaughton deleted the comment_add branch September 26, 2017 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants