search

s9y / Serendipity

A PHP blog software
https://s9y.org
BSD 3-Clause "New" or "Revised" License
203 stars 86 forks source link

Clean up the index.php #360

Closed onli closed 8 years ago

onli commented 9 years ago

A try to improve the routing, with the goal to use a routing framework in the end. See http://board.s9y.org/viewtopic.php?f=11&t=20467 for discussion and https://github.com/s9y/Serendipity/tree/feature_indexCleanup for the feature branch.

Please report clear bugs in that branch here.

garvinhicking commented 9 years ago

Wow, great work! This is a very promising start. I only read some code and didn't test it yet, but I like what I see.

One suggestion: I'd like to encapsulate functions_routing.php in a "serendipity_routing" class and maybe have static methods? Currently the "server*" functions don't really match our serendipity_XXX naming scheme; it helps to keep them short, but some sort of namespacing would help to make serendipity embeddable.

yellowled commented 9 years ago

Why create a seperate milestone for this and not just add it to x.0.0?

onli commented 9 years ago

Why create a seperate milestone for this and not just add it to x.0.0?

I want it for the next big version, not indefinitely, even if the next big version is not too near yet. x.0.0 didn't transport that for me.

I'd like to encapsulate functions_routing.php in a "serendipity_routing" class and maybe have static methods?

Certainly something to consider, but wait a bit with that. My goal is to use http://flightphp.com/ in the end, then we maybe do not need exactly that structure. Same for the naming scheme, this was by design to signal that it is temporary.

Great to hear positive feedback about this!

yellowled commented 9 years ago

I want it for the next big version

That is what x.0.0 is. A single milestone for one issue only (that's what it is as of now) seems very confusing and redundant.

onli commented 9 years ago

YL, don't be grumpy^^ Not worth it over bureaucracy. #187 is for me far more away than this, let's have fitting milestones for that. You can change one of the other issues if you think they are near enough as well.

More important: Have a look at https://github.com/s9y/Serendipity/commit/55ae435c898bfcff1a4722a34e25116188a1079c#diff-41936f33ec3468b12b1f74466f96a1d9L152 please. This PATH_AUTHORS stands also in 2.0 in the index.php, never gets defines as far as I can see, and becomes a part of the pagination link when selecting the author overview for multiple authors. I think that is a bug in master as well, and maybe you could try to see whether you can confirm that? Only if you have the time, if it is broken it is evidently not used anywhere.

yellowled commented 9 years ago

Creating a seperate milestone for a single issue makes organizing issues (which is what milestones are for) even more confusing than it already is. It just doesn't make any sense to me at all.

onli commented 8 years ago

I merged that now. But realized that this is a bigger task, already closed and re-opened – do we want to further work on this?

yellowled commented 8 years ago

@onli Isn't this in master already?

onli commented 8 years ago

It is, and the one issue we had with this so far got a separate issue. I'll close here.