Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs - 404/405 Route Deprecation Notice #173

Closed
Rican7 opened this issue Jan 16, 2014 · 7 comments · Fixed by #221
Closed

Docs - 404/405 Route Deprecation Notice #173

Rican7 opened this issue Jan 16, 2014 · 7 comments · Fixed by #221
Assignees
Milestone

Comments

@Rican7
Copy link
Member

Rican7 commented Jan 16, 2014

Since #161 created a new style of HTTP error handling, it makes sense to deprecate the old style with a deprecation notice of some kind...

Maybe like so?:

trigger_error(
    'Use of 404/405 "routes" is deprecated. Use $klein->onHttpError() instead.',
    E_USER_DEPRECATED
);
@ghost ghost assigned Rican7 Jan 16, 2014
@eimajenthat
Copy link
Contributor

Before you deprecate the old way, perhaps you should document the new way?

https://github.com/chriso/klein.php/wiki/Handling-404's

This still recommends creating a route with '404' as the pattern to match.

Will this change how we throw a 404 error in our routes? Like should we do throw HttpException::createFromCode(404) instead of $klein->abort(404) or something like that?

I see the correctness of making HTTP errors have their own handler, rather than being a route. I just want to know the plan so I can update my code. And I think others will as well.

@eimajenthat
Copy link
Contributor

I updated that wiki page based on the RFC myself. I figure you have enough on your plate. Maybe spot check my work?

I'd like to put something in about triggering errors, but I want to confirm whether the recommended technique is still to use $klein-abort() or not.

@Rican7
Copy link
Member Author

Rican7 commented Jan 21, 2014

I haven't deprecated the old way yet, and the newer way is yet to be released (its in master, but yet to be tagged), so I was going to wait and update the documentation later.

Having said that, I really appreciate you taking the time to update the Wiki! It looks great! Its a great example! It just needs a couple of corrections regarding the injected params.

I really appreciate you taking it upon yourself to make sure that the documentation was updated. That's awesome. I'll make the proper edits and will add the deprecation notice closer to the release of 2.1. Thanks again @eimajenthat!

@eimajenthat
Copy link
Contributor

Ah yeah, I ended up pulling dev-master on my project (was having some kind of issue with 2.0.x, though I can't remember what right now). Forgot other people might be running a prior version. Sorry about that. I'm thinking of writing some more docs for Klein when I have a chance. I'll see about submitting those through some other method, so you can deploy them at the appropriate time.

@Rican7
Copy link
Member Author

Rican7 commented Jan 22, 2014

No problem at all. That would be awesome! I'd really appreciate that! I've had some bad luck with people appreciating documentation lately: (#176)

Any help is greatly appreciated, and documentation is something I'm not amazing at, so it'd be a huge help. :)
Thanks again!

@eimajenthat
Copy link
Contributor

Regarding documentation, I was thinking a thought the other day. So, the thing about the Wiki is that there's only one. It's not tied to versions. I was considering writing docs as additional markdown files in a docs/ directory in the repo, so they can be versioned with the framework. I guess they'd be harder to find, but we could link to them from Wiki potentially. Just brainstorming right now. Your thoughts?

@Rican7
Copy link
Member Author

Rican7 commented Jan 22, 2014

That actually sounds like a great idea!
And if they ever get too big, we can always separate them into another repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants