luvit / lit

Toolkit for developing, sharing, and running luvit/lua programs and libraries.
http://lit.luvit.io/
Apache License 2.0
245 stars 58 forks source link

Add options table to request in coro-http #230

Closed JohnnyMorganz closed 4 years ago

JohnnyMorganz commented 6 years ago

I recently strolled upon a time where I had to use coro-http and not follow any redirects, but I realised that coro-http does not have this functionality built in.

This PR adds an options argument to the request function, which is a table of internal options With this, I also moved the timeout argument into the options table, which makes the table currently look like:

options
    timeout (number)
    noredirect (boolean)

This also allows more options to be added over time as required.

SinisterRectus commented 4 years ago

Sorry this PR has been ignored. I think an options table is a good idea, but the problem here is that this is a breaking change. It would require a major version bump. If we did do a major bump, I think it that it would be better to take advantage of a rewrite of some other parts of the library. An additional change I can think of is normalizing error handling across the coro libraries.

squeek502 commented 4 years ago

I also think an options table is a good idea. This can be made backwards-compatible as well by checking the type of the argument and treating it as timeout if it's a number.

SinisterRectus commented 4 years ago

Good point.

JohnnyMorganz commented 4 years ago

I had to create a new PR for this (https://github.com/luvit/lit/pull/278) because I have since deleted the branch this was worked on