← Back to team overview

kolibri-discuss team mailing list archive

Re: [Merge] lp:~asteinlein/kolibri/request+response into lp:kolibri

 

Review: Needs Fixing
Almost good to go. Only a few easily fixable errors discovered:
- RequestProcessor::throw404 is incorrect, tries to use NotFoundResult
- NotFoundResponse has a syntax error in parameterlist of constructor, and comment for status code is wrong
- In example wishlist app, some comments and one status message in actions refer to $this->request

And some suggestions while reading through changes, all of them not directly related, but:
- offsetX methods in both Request and Session should just return $this->get(…), $this->put(…) etc. instead of duplicating code
- Maybe drop getUri() and getMethod() completely in Request?
- AuthInterceptor has one call to $this->denyAccess() which sends $dispatcher as a parameter
- Class comment for Session should be updated to read that it's injected into the Request instance

There's also a few comments that still refer to result instead of response. Most notable Index action in wishlist app specifically naming XsltResult. And webRoot in wishlist config should be reverted to http://localhost :)

But all in all there's really not that much to fix before this can be merged into trunk!
-- 
https://code.launchpad.net/~asteinlein/kolibri/request+response/+merge/5529
Your team Kolibri Discuss is subscribed to branch lp:kolibri.



Follow ups

References