mafintosh / turbo-http

Blazing fast low level http server
MIT License
1k stars 47 forks source link

Add missing header features to work with fastify #18

Open tinchoz49 opened 6 years ago

tinchoz49 commented 6 years ago

Hi everyone! how are you?

@mafintosh this is what I did to test fastify using turbo-http.

The idea was to provide some methods for the request and response objects that are compatible with the official node module http.

Some notes:

  1. It's missing the tests for this methods but after we discuss this PR I'm going to add it :+1:
  2. I changed request.getAllHeaders() to return an object instead of a map to use the same method for the getter headers and be compatible with the http node module.
delvedor commented 6 years ago

I think you should add package-lock.json to the .gitignore :)

@mafintosh have you reviewed this? Can I help? :)

mafintosh commented 6 years ago

Just reviewed it! Sorry for the delay :)

tinchoz49 commented 6 years ago

haha don't worry, thanks for the review!

tinchoz49 commented 6 years ago

Hi @delvedor, how are you?

One question: Why in this case is wrong having versioned the package-lock.json?

delvedor commented 6 years ago

@tinchoz49 fine thanks! Because the package-lock.json is useful if you are writing an application, but if you are distributing a library not use it will allow you to fully embrace semver. In other words, your code will always have updated dependencies which should not break your code, and in some case even improve it :)

But at the end of the day it is a @mafintosh's choice :)

tinchoz49 commented 6 years ago

@delvedor it's true! I saw the same behavior using rust with cargo. Thanks for the explanation.

tinchoz49 commented 5 years ago

Hi @mafintosh! how are you? Did you have time to check the last changes and comments?

mafintosh commented 5 years ago

I'll take a look tmw. If not pls ping me :)

mafintosh commented 5 years ago

@tinchoz49 final review, thanks for your patience.

tinchoz49 commented 5 years ago

Thanks for the review @mafintosh! I replaced the forEach with for-of and I have a few questions about the getAllHeaders.

tinchoz49 commented 5 years ago

Hi @mafintosh ! did you have time to check the last changes on this?

dalisoft commented 5 years ago

ping