Added additional constructor to ParserSettings that complies with the old api

[tehrengruber]

Currently a lot of packages that rely on the HttpParser fail to build because of the backward incompatible changes in the signature of the ParserSettings constructor. I've added an additional constructor to ParserSettings with the old signature that prints a deprecated warning, but gives all packages that rely on HttpParser a grace period to update.

[coveralls]

Changes Unknown when pulling 6c4d618c54acc5ec1a0bc586d9bb5bf1caec8025 on tehrengruber:master into \ on JuliaWeb:master**.

[coveralls]

Changes Unknown when pulling 2b091a24293b56553b00114776812aa22442f77b on tehrengruber:master into \ on JuliaWeb:master**.

[tehrengruber]

I think the deprecation is useful. Currently a lot of packages just broke because of the signature change: e.g. HttpServer (PR pending), Requests (PR pending) and various other). After a given grace period we can remove the additional constructor safely.

[tehrengruber]

The syntax works without problems on 0.4 as you can see in the CI output

[yuyichao]

What I mean is that this doesn't need to be deprecated.

[tehrengruber]

Thats right, but then we should make all callback arguments optional to be consistent.

[yuyichao]

but then we should make all callback arguments optional to be consistent.

If any of them are reasonably optional, I think that's a good idea.

[yuyichao]

julia> cfunction(()->1, Int, Tuple{})
ERROR: only generic functions are currently c-callable
 in cfunction at ./c.jl:9

No depwarn is printed on the CI so I don't think it's covered.

[tehrengruber]

I've added another version where every callback is optional. Generally I think that all of the arguments are optional, since users of the library might only be interested in some of the callbacks. The cleanest solution would be to have the default constructor, a constructor with keyword arguments (if that works) and another constructor that is compatible with the old api but deprecated. What do you think?

[coveralls]

Changes Unknown when pulling 88ee6d7ec0524288cb3a136e4f035a1ac904fe7a on tehrengruber:master into \ on JuliaWeb:master**.

[yuyichao]

a constructor with keyword arguments (if that works) and another constructor that is compatible with the old api but deprecated.

I think that could work. (I'm personally not a big fan of kw arguments before they become fast but I guess that isn't a good reason for not using it at all....)

[yuyichao]

And if this includes a API breakage (edit: i.e. use kwargs instead) (even though it comes with deprecated old version) I'd like to hear opinions from other members before merging.

[coveralls]

Changes Unknown when pulling ae7509e794be4c09b5e8e7c812b199b1d8a9eeef on tehrengruber:master into \ on JuliaWeb:master**.

[tehrengruber]

And I believe http_cb and http_data_cb need two different dummies (or two methods of it at least).

Absolutely I've changed that accordingly.

The last commit is what I've proposed previously

Generally I think that all of the arguments are optional, since users of the library might only be interested in some of the callbacks. The cleanest solution would be to have the default constructor, a constructor with keyword arguments (if that works) and another constructor that is compatible with the old api but deprecated. What do you think?

We now have a constructor that complies with the old api, but which is deprecated and another constructor that accepts keyword arguments. By doing so the current API is preserved, the old api is still usable but deprecated and we have a new constructor for people that only use some callbacks (unused for now).

Edit: We might want to rebase all of the commits in this pull request to keep the commit history clean.

[coveralls]

Changes Unknown when pulling 07540f6c205d06098d23f942d2252b3c0c08e996 on tehrengruber:master into \ on JuliaWeb:master**.

[coveralls]

Changes Unknown when pulling 969e9958e53b73df0eb26164b342b7f288cd0ec8 on tehrengruber:master into \ on JuliaWeb:master**.

[tehrengruber]

Ok I think we finally made it :-) Any further comments & suggestions?

[yuyichao]

The change LGTM now, I'll let others comment on what the best interface should be (and therefore, if these should be inner constructors).

The added constructor should be tested though.

[aviks]

Sorry this never got merged. Is this still relevant?