[wp-trac] [WordPress Trac] #11644: multiple blogs & sites / merge WPMU

Wed Feb 24 12:25:10 UTC 2010

#11644: multiple blogs & sites / merge WPMU
----------------------------+-----------------------------------------------
 Reporter:  hakre           |       Owner:  wpmuguru 
     Type:  task (blessed)  |      Status:  assigned 
 Priority:  normal          |   Milestone:  3.0      
Component:  Multisite       |     Version:           
 Severity:  normal          |    Keywords:  multisite
----------------------------+-----------------------------------------------

Comment(by hakre):

 Replying to [comment:195 nacin]:
 > Running down hakre's list, as handled in [13343]: ...

 Thanks for the feedback. For the version number lookup, that's a job I'll
 do, no need for you to care. I thought for the same for sorting it, I just
 wanted to tell you about the Idea. But that's fine anyway now, so I do not
 need to do it.

 Here is a new numbered list, some older unfixed are still on it, just
 ignore if you do not want to care, I keep them for reference. There are
 three or four more major points left over, for other just give me a hint
 what you prefer, sometimes I asked questions, would be great if you can
 answer them so I know better for upcomming patches we do together.

  1. UNFIXED {{{__destruct()}}} breaks with PHP5 defition of the return
 value (should be void, is true)
  2. UNFIXED: in the variable definitions, the meta tables sometimes
 precede their main table. Also tables are not sorted by their names. Let
 me know if you would like to see that fixed.
  3. UNFIXED: I'm not sure wether unsetting members of an array while
 iterating it is save with all the PHP version we aim to {{{unset( $tables[
 $k ] );}}}.
  4. QUERSTION: you've set access to {{{wpdb::$blogid}}} to public. why?
 should other parts of the program overwrite the blogid for a certain
 functionality? Same for {{{wpdb::$siteid}}}. A public setter for both is
 {{{wpdb::set_blog_id()}}}. Maybe a get_blog_id() function with an optional
 parameter so that it's a getter for siteid as well?
  5. QUESTION: you skip the note about the problem of registering the wpdb
 instance with the register shutdown function call prevents it from
 unloading as expected. What's the problem with leaving a note there? Same
 for the hints given in wpdb::__desctruct(). I found those thoughtfull so
 we at least have the discrepancies documented.
  6. QUESTION: What's the problem with the scope 'now+old'? I mean there
 are at least two points in wpdb that will benefit from it.
  7. QEUSTION: What's the problem with listing the scope values inside the
 docblock. I think that's easier to read than inside the @param
 description. And easier to maintain.
  8. wpdb::tables() did not return an array as specified for certain scope
 values (the undefined ones were forgotten). Patched, will prevent
 notices/warnings inside the function as well.
  9. There is no need to cast an array as array. wpdb::tables() always
 returns an array, you know that - well okay, not it is.
  10. Since you did not want to have the merge inside wpdb::tables() (for a
 reason I do not understand), array_merge still actually works. I prefer to
 encapsulate that inside tables(), really.
  11. All parameters in tables() are optional. @param should reflect that.
  12. Scopes can be sorted by alphabet in the switch construct in
 wpdb::table()
  13.
 [http://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutorial_tags.uses.pkg.html
 @uses] does not fit here, please use @see as already suggested in
 {{{wpdb::_weak_escape()}}}.
  14. INFO: for the same class you do not need to prefix wpdb:: for @see
 and @uses. Additionally PHP function names can be used with @see as well.
  15. QUESTION: Sometimes @-tags are aligning their values, sometimes not.
 Any Idea? Normally alignment is missing in core code, so with these
 iteration I removed those where I saw them.
  16. MINOR: If in for single quotes on 'foo', should be in for single
 quotes on 'SELECT... as well, right?
  17. There is a problem with the preg_replace call in wpdb::prepare()
 reported on another ticket. I suggest to keep the old behaviour.
  18. QUESTION: Since when is $this always a reference? Since the beginning
 or has that changed later?
  19. SKIPPED: I did not care about error_log now. I'll do that later. I
 saw you changed something there, my Idea was to check against the base
 reference. I suggest to keep that the old code until we have fully
 clarified that to not break behaviour.
  20. It's toally clear that some queries are made prior to load the plugin
 API. No need to comment, right? Or if so, why not comment the other parts
 I find totally clear as well (e.g. preventing the unload)
  21. INFO: {{{wpdb::insert( 'table', array( 'column' => 'foo', 'field' =>
 1337 ) )}}} was the original example. I just added one, I just saw no need
 to edit it (FYI).
  22. INFO: I still think that constants are normally written uppercase,
 but I dropped that for this patch to reduce the FUZZ. This needs to be
 clarified elsewere anyway I assume.
  23. There still was a parameter documented that does not exists in
 wpdb::has_cap(), you've forgotten a line to remove or was that by
 intention?
  24. You made a mistake in changes applied to wpdb::get_caller(), $caller
 is not filled with values any longer. A line was missing. You can expand
 it if you like using a temporary variable like $function, but I just
 patched it.
  25. Additionally I moved the join next to return (as always suggested) in
 {{{wpdb::get_caller()}}}.
  26. Another parameter that does not exists documented in
 {{{wpdb::db_version()}}} (see 23.)
  27. QUESTION: Include files to not need control characters after the
 closing {{{?>}}} at the end, right?

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/11644#comment:197>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software