Refactor author usage into WP_Stream_Author class; use System user always #448
Conversation
| } elseif ( ! empty( $this->meta['user_login'] ) ) { | ||
| return $this->meta['user_login']; | ||
| } else { | ||
| return __( 'N/A', 'stream' ); |
There was a problem hiding this comment.
@westonruter Should this also be "Unknown"?
| ); | ||
| } | ||
|
|
||
| $author_obj = new WP_Stream_Author( $user_id ); |
There was a problem hiding this comment.
Personally, I prefer to not include the variable type in it's name (for example, I wouldn't name a string $example_string). How would you feel about changing this to $author?
WordPress does this with things like $post or $user.
There was a problem hiding this comment.
Just realised we're already using $author. Should this be changed to $user?
There was a problem hiding this comment.
But id is not a type. It could be anything. And I want to distinguish between the variable containing a user object vs the underlying user ID.
There was a problem hiding this comment.
Sorry - I meant the variable $author_obj, where object is the type.
|
@westonruter @fjarrett I'm happy with this and ready to merge, but I made a few minor changes in my last commit. I'd appreciate another set of eyes and opinions if possible. 😁 |
| $agent = 'wp_cron'; | ||
| } | ||
|
|
||
| $agent = apply_filters( 'wp_stream_current_agent', $agent ); |
There was a problem hiding this comment.
@westonruter This hook needs a docblock, as well as wp_stream_agent_label below.
There was a problem hiding this comment.
Went ahead and merged anyway. Will still need to do this later.
|
@westonruter Also curious about the new |
|
It is the convention in Core. |
|
So we should rename the others to match the Core convention? |
|
Eventually, sure. Will make autoloading easier. |
Refactor author usage into WP_Stream_Author class; use System user always
@lukecarbis please review