Merge lp://staging/~anandjeyahar/beeseek/Fedora12 into lp://staging/beeseek

First of all, thanks for your contribution.

Your branch is good: the code you have changed works as it should and does it jobs. However there are few problems with it. The most important thing that you should do is fixing the tests: in fact, if you run "make check" or just "tools/test-node", you'll notice some failures introduced by your changes. You won't have to change nothing special: just 'USER-AGENT' -> 'HTTP_USER_AGENT' and so on, so it shouldn't be difficult to fix the tests, but please tell me if you have troubles.

There are also a few minor problems with your code:

* You placed the "name = 'HTTP_' + name.replace('-', '_')" inside the try..except block. If an error occurs while concatenating 'HTTP_' with `name`, than it means that there is a bug in our software that we should fix. The error shouldn't pass silently, but instead must raise an exception. It means that you should place the code out of the try..except block.

* Could you please replace 'name.replace(...)' with 'name.upper().replace(...)' (removing the call to upper() in the line below)? This will make things a bit faster (although not significantly), since the prefix 'HTTP_' won't be touched by upper().

* In webserver.py you removed the TODO and the `name = ...` line, and that's good. But with this change now the loop is totally useleess: you can use the update() method of the request_meta dictionary: request_meta.update(headers).

This last three problems are actually just nitpicks, but I would like to see the tests fixed before approving the merge.

Thanks,
Andrea

Oh i had run link and make install.
Both went through, so i just committed. Will fix these and post again.
==============================================
Anand Jeyahar
http://sites.google.com/a/cbcs.ac.in/students/anand
==============================================
The man who is really serious,
with the urge to find out what truth is,
has no style at all. He lives only in what is.
                  ~Bruce Lee

Love is a trade with lousy accounting policies.
                 ~Aang Jie

On Mon, Aug 30, 2010 at 15:11, Andrea Corbellini <
<email address hidden>> wrote:

> Review: Needs Fixing
> First of all, thanks for your contribution.
>
> Your branch is good: the code you have changed works as it should and does
> it jobs. However there are few problems with it. The most important thing
> that you should do is fixing the tests: in fact, if you run "make check" or
> just "tools/test-node", you'll notice some failures introduced by your
> changes. You won't have to change nothing special: just 'USER-AGENT' ->
> 'HTTP_USER_AGENT' and so on, so it shouldn't be difficult to fix the tests,
> but please tell me if you have troubles.
>
> There are also a few minor problems with your code:
>
> * You placed the "name = 'HTTP_' + name.replace('-', '_')" inside the
> try..except block. If an error occurs while concatenating 'HTTP_' with
> `name`, than it means that there is a bug in our software that we should
> fix. The error shouldn't pass silently, but instead must raise an exception.
> It means that you should place the code out of the try..except block.
>
> * Could you please replace 'name.replace(...)' with
> 'name.upper().replace(...)' (removing the call to upper() in the line
> below)? This will make things a bit faster (although not significantly),
> since the prefix 'HTTP_' won't be touched by upper().
>
> * In webserver.py you removed the TODO and the `name = ...` line, and
> that's good. But with this change now the loop is totally useleess: you can
> use the update() method of the request_meta dictionary:
> request_meta.update(headers).
>
> This last three problems are actually just nitpicks, but I would like to
> see the tests fixed before approving the merge.
>
> Thanks,
> Andrea
> --
> https://code.launchpad.net/~anandjeyahar/beeseek/Fedora12/+merge/34012
> You are the owner of lp:~anandjeyahar/beeseek/Fedora12.
>

Hi, remember to bzr commit and push when you are done. ;-)

I have modified the doc examples according to the new change. So no errors from there, BUT there are still 10 errors. I just don't have the time at the moment(my lack of understanding with web programming and my change of job.) Thought you might be able to fix it sooner.

OK, no problem. I've fixed the tests and uploaded your changes (rev 100 and 101 of lp:beeseek).