Code review comment for lp:~uriboni/oxide/basic-authentication

Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Thanks for this - I've left some comments inline.

One general point - there's a mixture of 2-space and 4-space indentation. Please stick to 2-space (using 4-space only for wrapped lines). We try to adhere to the Chromium style guide on this - see http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Spaces_vs._Tabs.

Also, what happens if the application doesn't connect a handler, or does connect one but allows the request object to be garbage collected before responding to it? Oxide should take the default action - ie, deny - but it doesn't look like that will happen here. Please take a look at how this is handled for permission requests, and there should also be a test case for this as well.

review: Needs Fixing

« Back to merge proposal