OMG open source contributions!

eteeselink commented 13 years ago

Hi Jose,

I really like Anna, so I spent some saturday time on hacking on it. Things I added / changed:

I added a GetBody() method to Request, which asynchronously reads the input stream into a string (in a single go, so not very suited for large uploads, but very handy for HTML form posts and RESTful APIs and the likes). I guess it can be improved a lot, but hey, it's a start.
I must admit that I didn't really like the way responses were implemented, with the implicit conversions and all: these are very difficult to discover from the IDE. You need to read examples or the source to see how to use it. So I rewrote it to simple methods on RequestContext.
- I added a Send method to the Response class. Advantage is that logically, I believe that this code should be local to the class. Disadvantage is that now Response has a dependency on RequestContext (which is passed through its constructor).
- I added some overloaded factory methods called Response and StaticFileResponse that create the right response object based on the parameter types. Users can thus call e.g. context.Response("moo").Send()
- Changed some Respond methods so that they take literal strings and ints instead of implicitly converted Response objects, so that they are discoverable from the IDE.
- Removed the implicit conversions
- Result is that a user has the same options as before, but can type context. and a drop-down with all the different ways to create responses is shown by the IDE.
Added some doc comments here and there, plus a simple readme with an example

No idea whether you're open to this at all. Curious to see what you think!

jfromaniello commented 13 years ago

Hi Egbert! Thank you very much for your interest, I am very open to contributions so I will have a look to your changes ASAP I like some of the changes that I saw!

thanks again

2011/10/8 Egbert Teeselink < reply@reply.github.com>

Hi Jose,

I really like Anna, so I spent some saturday time on hacking on it. Things I added / changed:

I added a GetBody() method to Request, which asynchronously reads the input stream into a string (in a single go, so not very suited for large uploads, but very handy for HTML form posts and RESTful APIs and the likes). I guess it can be improved a lot, but hey, it's a start.

I must admit that I didn't really like the way responses were implemented, with the implicit conversions and all: these are very difficult to discover from the IDE. You need to read examples or the source to see how to use it. So I rewrote it to simple methods on RequestContext.

I added a Send method to the Response class. Advantage is that logically, I believe that this code should be local to the class. Disadvantage is that now Response has a dependency on RequestContext (which is passed through its constructor).

I added some overloaded factory methods called Response and StaticFileResponse that create the right response object based on the parameter types. Users can thus call e.g. context.Response("moo").Send()

Changed some Respond methods so that they take literal strings and ints instead of implicitly converted Response objects, so that they are discoverable from the IDE.

Removed the implicit conversions

Result is that a user has the same options as before, but can type context. and a drop-down with all the different ways to create responses is shown by the IDE.

Added some doc comments here and there, plus a simple readme with an example

No idea whether you're open to this at all. Curious to see what you think!

You can merge this Pull Request by running:

git pull https://github.com/eteeselink/Anna master

Or you can view, comment on it, or merge it online at:

https://github.com/jfromaniello/Anna/pull/1

-- Commit Summary --

Changing response API to IDE-discoverable method interface: compiles

Added docs to response code; added a test; removed unused code; added status codes to Response child class constructors; I should really learn to commit more often.

Added Request.GetBody(), a simple non-chunked input stream reader for reading POSTed string data.

Added example code and simple README.md

-- File Changes --

M Anna.Tests/ResponsesTests.cs (28) M Anna.Tests/ServerTests.cs (89) M Anna.Tests/Util/Browser.cs (17) M Anna/HttpServer.cs (3) M Anna/Observers/UnhandledRouteObserver.cs (2) M Anna/Request/Request.cs (23) M Anna/Request/RequestContext.cs (90) M Anna/Responses/BinaryResponse.cs (4) M Anna/Responses/EmptyResponse.cs (5) M Anna/Responses/Response.cs (49) M Anna/Responses/StaticFileResponse.cs (3) M Anna/Responses/StringResponse.cs (5) A README.md (24)

-- Patch Links --

https://github.com/jfromaniello/Anna/pull/1.patch https://github.com/jfromaniello/Anna/pull/1.diff

Reply to this email directly or view it on GitHub: https://github.com/jfromaniello/Anna/pull/1

eteeselink commented 13 years ago

Hi José!

Cool to hear that! :-) No rush, btw.

That said, did you by any chance get that other email I sent you about
Anna's license?

Egbert

eteeselink commented 13 years ago

In other news: Collaborating on GitHub is fairly new for me. I have the underbelly feeling that I should've made this multiple little pull requests but didn't manage. Any feedback on the process would be most appreciated as well :-)

jfromaniello commented 13 years ago

I don't remember to read any mail about the license... I guess MIT http://www.opensource.org/licenses/mit-license.php will be okay, but if you need any other kind of license type just let me know. This started as an experiment and I can adopt anything.

About collaborating in github I am new too , but the recommended approach is to:

pull
open a branch for the new feature you will work on
commit, commit, work rever,t commit, etc, etc..
PUSH with the branch opened..

And then i will pull and merge your branch.

But this time do not worried at all.

2011/10/8 Egbert Teeselink < reply@reply.github.com>

Hi Jos!

Cool to hear that! :-) No rush, btw.

That said, did you by any chance get that other email I sent you about Anna's license?

Egbert

Reply to this email directly or view it on GitHub: https://github.com/jfromaniello/Anna/pull/1#issuecomment-2332702

eteeselink commented 13 years ago

Hm, then I must've mailed to some wrong address. I'll look it up at work
on monday.

MIT looks fine. If you'd care to publish it on the github repos, I'll
probably end up using Anna for a small project at work.

Thanks for the little heads-up on the process. Looks like I did all that,
except for the feature branch. Will do that in the future.

Egbert

On Sat, 08 Oct 2011 20:25:21 +0200, José F. Romaniello
reply@reply.github.com
wrote:

I don't remember to read any mail about the license... I guess MIT http://www.opensource.org/licenses/mit-license.php will be okay, but if
you need any other kind of license type just let me know. This started as an experiment and I can adopt anything.

About collaborating in github I am new too , but the recommended
approach is to:

pull

open a branch for the new feature you will work on

commit, commit, work rever,t commit, etc, etc..

PUSH with the branch opened..

And then i will pull and merge your branch.

But this time do not worried at all.

2011/10/8 Egbert Teeselink < reply@reply.github.com>

Hi Jos!

Cool to hear that! :-) No rush, btw.

That said, did you by any chance get that other email I sent you about Anna's license?

Egbert

Reply to this email directly or view it on GitHub: https://github.com/jfromaniello/Anna/pull/1#issuecomment-2332702

jfromaniello commented 13 years ago

I merged your changes! thank you very much for all your contributions. I also pushed a new version to nuget, let me know any other feedback, stay in touch.

2011/10/8 Egbert Teeselink < reply@reply.github.com>

Hm, then I must've mailed to some wrong address. I'll look it up at work on monday.

MIT looks fine. If you'd care to publish it on the github repos, I'll probably end up using Anna for a small project at work.

Thanks for the little heads-up on the process. Looks like I did all that, except for the feature branch. Will do that in the future.

Egbert

On Sat, 08 Oct 2011 20:25:21 +0200, Jos F. Romaniello reply@reply.github.com wrote:

I don't remember to read any mail about the license... I guess MIT http://www.opensource.org/licenses/mit-license.php will be okay, but if you need any other kind of license type just let me know. This started as an experiment and I can adopt anything.

About collaborating in github I am new too , but the recommended approach is to:

pull

open a branch for the new feature you will work on

commit, commit, work rever,t commit, etc, etc..

PUSH with the branch opened..

And then i will pull and merge your branch.

But this time do not worried at all.

2011/10/8 Egbert Teeselink < reply@reply.github.com>

Hi Jos!

Cool to hear that! :-) No rush, btw.

That said, did you by any chance get that other email I sent you about Anna's license?

Egbert

Reply to this email directly or view it on GitHub: https://github.com/jfromaniello/Anna/pull/1#issuecomment-2332702

Reply to this email directly or view it on GitHub: https://github.com/jfromaniello/Anna/pull/1#issuecomment-2333351

eteeselink commented 13 years ago

Woa, happy to hear that! :-)

I might do more contributions in the near future, but not sure yet. I've a small contribution ready already on my github (some additional doc comments), but I'll wait until I've added more docs than that.

Btw, did I break that test you fixed? Oops!

On 17.10.2011 17:58, José F. Romaniello wrote:

I merged your changes! thank you very much for all your contributions. I also pushed a new version to nuget, let me know any other feedback, stay in touch.

2011/10/8 Egbert Teeselink < reply@reply.github.com>

Hm, then I must've mailed to some wrong address. I'll look it up at work on monday.

MIT looks fine. If you'd care to publish it on the github repos, I'll probably end up using Anna for a small project at work.

Thanks for the little heads-up on the process. Looks like I did all that, except for the feature branch. Will do that in the future.

Egbert

On Sat, 08 Oct 2011 20:25:21 +0200, Jos F. Romaniello reply@reply.github.com wrote:

I don't remember to read any mail about the license... I guess MIT http://www.opensource.org/licenses/mit-license.php will be okay, but if you need any other kind of license type just let me know. This started as an experiment and I can adopt anything.

About collaborating in github I am new too , but the recommended approach is to:

pull

open a branch for the new feature you will work on

commit, commit, work rever,t commit, etc, etc..

PUSH with the branch opened..

And then i will pull and merge your branch.

But this time do not worried at all.

2011/10/8 Egbert Teeselink < reply@reply.github.com>

Hi Jos!

Cool to hear that! :-) No rush, btw.

That said, did you by any chance get that other email I sent you about Anna's license?

Egbert

Reply to this email directly or view it on GitHub: https://github.com/jfromaniello/Anna/pull/1#issuecomment-2332702

Reply to this email directly or view it on GitHub: https://github.com/jfromaniello/Anna/pull/1#issuecomment-2333351

Anna-Rx / Anna

OMG open source contributions! #1