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 count_users
out of roles foreach
#964
Conversation
classes/class-settings.php
Outdated
@@ -817,13 +817,14 @@ public function render_field( $field ) { | |||
// Author or Role dropdown menu | |||
$author_or_role_values = array(); | |||
$author_or_role_selected = array(); | |||
$users = count_users(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@frozzare You can move this even higher up, outside the foreach
of the exclude_row
. Perhaps place it at the beginning of the rule_list
case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's all cool. If you could move that a little higher to be optimized more, then we're all good.
Done! |
classes/class-settings.php
Outdated
@@ -771,6 +771,7 @@ public function render_field( $field ) { | |||
|
|||
break; | |||
case 'rule_list': | |||
$users = count_users(); | |||
$form = new Form_Generator(); | |||
$output = '<p class="description">' . esc_html( $description ) . '</p>'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Could you please align the =
so the build can pass ;)
classes/class-settings.php
Outdated
@@ -823,7 +824,7 @@ public function render_field( $field ) { | |||
'value' => $role_id, | |||
'text' => $role, | |||
); | |||
$users = count_users(); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, clear up this line ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@frozzare if you can address those two formatting issues, we'll be good to go.
This pull request will move
count_users
out of roles foreach since it's expensive to call for every role. See #949 for more information.This time against develop instead of master.