[buddypress-trac] [BuddyPress] #4140: URI page router
buddypress-trac at lists.automattic.com
buddypress-trac at lists.automattic.com
Thu Apr 12 12:04:16 UTC 2012
#4140: URI page router
-------------------------+-----------------------------
Reporter: foxly | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: Future Release
Component: Core | Version:
Severity: major | Resolution:
Keywords: 1.7-early |
-------------------------+-----------------------------
Changes (by boonebgorges):
* keywords: => 1.7-early
* milestone: Awaiting Review => Future Release
Comment:
Thanks, Foxly.
I've spent a bit of time with the patch. Before I level criticism :) let
me say that the general architecture of this router class is far, far more
sane than the hodge-podge currently in BP. I think it represents a very
nice foundation for moving forward.
In order to get it running on my setup, I had to correct a couple of
problems:
- The logic that accounted for WP running in a subdirectory
(example.com/wp/) was missing. It's possible that it was incorrectly
ported because the original parser is a little redundant in this respect,
and has unclear docs.
- $wp_query->is_singular had to be set to true in order for most pages not
to 404
- Cleared up a number of smallish PHP notices
The first of these errors (not running properly in a subdirectory) points
toward the importance of having automated tests for this new class. For
all of the bad things you might say about the existing
`bp_core_set_uri_globals()` - and there are plenty of bad things to say! -
it had been battle tested (if not unit tested) to account for lots and
lots of different scenarios. It's imperative to make sure that they've
carried over. I'm looking forward to getting my hands dirty writing some
tests to cover these cases, but I'll wait until you guys have shared
yours, so that we don't duplicate effort.
There are some details that we'll need to hash out. Code format (naming
conventions, comment format, etc) should be brought into line with WP/BP
standards. I'll want to talk more about the error/status reporting system:
I would like to avoid, if possible, introducing our own bespoke system
when we have something more general like WP_Error available, though you've
suggested that your format is part of the unit testing framework, so we'll
have to discuss how to move forward with it.
BP 1.6 is far overdue. I want to get it out, and adding these changes to
the release has the potential to extend the cycle by a lot. So I'm moving
it to Future Release with a 1.7-early tag. It's one of the two or three
things I'll be most interested in tackling for the next dev cycle, and
I'll keep playing with it in the meantime.
4140.02.patch is a proper patch against the current trunk (r5970), with
some of the fixes above. I'm going to spend more time with it in the
upcoming weeks or two, to test some of the benchmark claims that Foxly put
in the original ticket :) Again, thanks to the BP Media team, and thanks
in advance to all for your feedback and help in refinement.
--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/4140#comment:5>
BuddyPress <http://buddypress.org/>
BuddyPress
More information about the buddypress-trac
mailing list