[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