Should we load admin class only if is_admin() ?

[tommcfarlin]

This was something that was originally done by @grappler. In fact, the code was:

if ( is_admin() ) {
    add_action( 'plugins_loaded', array( 'Plugin_Name_Admin', 'get_instance' ) );
}

But I removed the conditional because the idea behind the entire admin class is that it will only be using admin hooks; therefore, nothing in the object will execute unless its loaded in the dashboard.

If this is something that the larger community believes to be a better option, then I'll happily reinstate it; however, it seems redundant to me.

[tivnet]

It should be a minor issue, of course. I just believe that it's safer not to execute any of the admin class code if not is_admin. And we do not need the instance either. Having that if sounds good to me.

[leewillis77]

There's two points here:

1. Since we're not doing autoloading there's an advantage (Memory, disk IO etc.) to be had by not even loading the files - even if they're a no-op functionally speaking. @grappler's code that you quoted doesn't do that it just avoids "executing the code". We should try and avoid even including the file [IMHO]
2. For info - you can't rely on is_admin() to detect if this is an "admin request" since "frontend" AJAX requests that go through the standard WordPress handler (admin-ajax.php) will return true for is_admin(). The suggested workaround would be:

if ( is_admin() && ( !defined( 'DOING_AJAX' ) || !DOING_AJAX ) )

See: http://yoast.com/separate-frontend-admin-code/

Although that will cause problems for any admin-side AJAX requests - so I think sticking with just is_admin() probably makes sense - just be aware that the admin file will still be included for front-end AJAX requests (Which ironically you probably want to be as light-footprinted as possible - Insert rant about lack of a front-end AJAX handler in WP ....)

[tivnet]

@leewillis77 :+1:

[tommcfarlin]

Sold on this. Will be assigning this to 2.6.0 milestone.

Thanks guys :).

[grappler]

@leewillis77 FYI - That is what I had too. https://github.com/tommcfarlin/WordPress-Plugin-Boilerplate/commit/ac8e5cfb01478f67f31142da5c9f45449b7186ef

[tivnet]

@tommcfarlin :+1: Do you want to require_once also conditionally?

And what about: "Although that will cause problems for any admin-side AJAX requests..." -- @leewillis77

[tommcfarlin]

I'm still working on a little bit of refactoring.

As far as the ajax dilemma is concerned, it's a bit of a catch 22. I'm apt to stick to with how we have it right now and leave a TODO saying that if you don't want include Ajax in your admin, then remove it.

[tommcfarlin]

Fixed with this commit.

[tivnet]

I am still thinking about #105 ... Where will the "common" part go, to the "public"?

[tommcfarlin]

I'm still thinking about it myself. We'll have to discuss this further as we prepare for 2.7.0.

I'm aiming to get 2.6.0 out soon after which we can revisit some of these.

[grappler]

@tommcfarlin Version 2.6.0 was released on 7th July.

[tommcfarlin]

I'm in the process of updating the change log for this release. I talk a bit more about the versioning scheme in this post.

The short of it is this:

• I've been doing a poor job of versioning the boilerplate
• Now that there's a lot of activity and discussion happening around the project, I need to do a better job of managing it.
• Since 2.5.1 was the last tagged version, we are starting this new versioning with 2.6.0.

It's confusing, but I'd like to get it on track now rather than later so that we have proper milestones moving forward, and its easier for newer contributors to catch up.

I know it's a little confusing, but such is maintaining projects early on ;).