search

WordPoints / wordpoints

Points plugin for WordPress
GNU General Public License v2.0
20 stars 15 forks source link

Deprecate debug functions #70

Closed JDGrimes closed 10 years ago

JDGrimes commented 10 years ago

The wordpoints_debug_message() and wordpoints_debug() functions should probably be deprecated. They currently aren't doing much of use. WordPress doesn't exactly have this kind of debugging function, so it makes our error handling/debugging style different than WordPress core. That's something we should avoid unless we have a really good reason.

Also, we are only using this debugging method in a small portion of the code (related to WordPoints_Points_Logs_Query. So we aren't being internally consistent either.

Another problem is that their usage is borderline error handling/debugging. Which is it supposed to be? Ideally, if somebody is really _doing_it_wrong(), we should tell them, and if there is an error we should use WP_Errors. Otherwise, the error should be ignored, or false should be returned.

JDGrimes commented 10 years ago

I'm backtracking on this a little bit. Several thoughts:

  1. There are a few places we call wp_debug_message() that we should just let PHP give an error if there is a problem. These calls should definitely be removed.
  2. There may be one or two places where we could return a WP_Error. One issue is back-compat, however.
  3. Most of the places this function is used aren't actually returning something to the initiating code, so a WP_Error is out. Moving to _doing_it_wrong() is one possibility for these, but: (1) they aren't all exactly the same spirit as what _doing_it_wrong() is about (calling a function incorrectly [like at the wrong time]), (2) _doing_it_wrong() doesn't provide quite as much information as wp_debug_message() does, so it isn't as helpful by default (for some cases), and (3) _doing_it_wrong() was originally intended to be private to WordPress core.

Let's focus on that third group. The first objection is really not that important, IMO. Same for the third objection; many plugins use _doing_it_wrong(), so it's sort of been adopted by the public at this point, even if against it's intended function. The second objection may hold a little merit, but really, knowing the line number isn't that important. The thing that is good to know sometimes is the file, but that can be obtained by tracing with _doing_it_wrong(), so I guess it isn't a big deal.

So on second thought, the first and third objections are the ones I think we should consider. The question is, should we consider _doing_it_wrong() as a public debugging API for WordPress. That wasn't it's original intention, but it is being used that way. Next question, is WordPress going to ever implement a public debugging API? I think not, but maybe it deserves research.

So, for now, fix the first group, and do research on what to do with group 3. Once group 1 is done I'll think more about the back-compat issues with group 2.

TL;DR: Maybe this should just be an audit of the function's use, and not a time to deprecate it.

JDGrimes commented 10 years ago

Searched through all of the tickets on WP Core Trac, and found:

So I think I'm leaning back toward deprecation. Using _doing_it_wrong() has the added benefit that any debugging plugins which are hooking into it will automatically pick up our messages as well. That seems the best way to go.