[wp-meta] [Making WordPress.org] #1113: Site Cloner: Add support for Remote CSS plugin
Making WordPress.org
noreply at wordpress.org
Fri Nov 18 20:25:56 UTC 2016
#1113: Site Cloner: Add support for Remote CSS plugin
--------------------------------------+-----------------------
Reporter: iandunn | Owner: iandunn
Type: enhancement | Status: accepted
Priority: normal | Milestone:
Component: WordCamp Site & Plugins | Resolution:
Keywords: good-first-bug has-patch |
--------------------------------------+-----------------------
Comment (by iandunn):
That looks great, thanks Corey :)
The only things I might change would be a couple small readability things:
1. If you `use \WordCamp\RemoteCSS;` at the start of the file, you can
then call `RemoteCSS\get_safe_css_post` instead of
`\WordCamp\RemoteCSS\get_safe_css_post`
2. It's not obvious why was the `str_replace` is needed, so it might be
worth leaving a comment to explain. It looks like Jetpack doesn't comment
it either, though. Was that just a precaution, or did you run into a
specific problem? Either way it'd probably be good to document that in the
code.
3. You can avoid some of the nesting by returning early, which makes it
more readable. Same thing for the `instanceof` check. It is nice to have a
single point of return, but I think it's ok to have multiple points
[https://tommcfarlin.com/wordpress-refactoring-plugin-
functions/#comment-40522 as long as they're at the very start of the
function].
{{{
if ( function_exists( '\WordCamp\RemoteCSS\get_safe_css_post' ) ) {
return $remote_css;
}
}}}
Those are obviously small things, though, and open to interpretation, so
feel free to use your judgement and go with what you think is best.
--
Ticket URL: <https://meta.trac.wordpress.org/ticket/1113#comment:4>
Making WordPress.org <https://meta.trac.wordpress.org/>
Making WordPress.org
More information about the wp-meta
mailing list