Tectu / malloy

A cross-platform C++20 library providing embeddable server & client components for HTTP and WebSocket.
BSD 3-Clause "New" or "Revised" License
66 stars 8 forks source link

Release v0.4.0 #104

Closed Tectu closed 2 years ago

Tectu commented 2 years ago

We've quite a lot of breaking changes coming up due to the (very needed) #102.

@0x00002a I'd like to discuss what other changes we might want to incorporate for the next release.

For one we do have #74 / #97 still open. Currently I have a hard time guesstimating whether we should get that into the next release. I fail to get a good metric of how likely it is that this change will lead to unexpected side effects which might need some really proper testing first.

Issue #77 is still a thing for me. I can do without but I feel like there should be a way of doing this more or less elegantly without a lot of overhead. I have a mostly working design for this but there are a few rough edges / edge cases I want to investigate further.

Issue #35 is not gonna happen any time soon IMHO.

Issue #46 is mostly out of our hands and indirectly related to #74 / #97 so I leave that particular issue out of this discussion.

Issue #89... I don't know what to say. I spend what feels like 10 hours on this and I am getting nowhere. My hunch is that it's related to dynamic memory allocation/deallocation going wrong somewhere but I don't have any hard evidence to support this.

Issue #95 is an easy fix so that will probably make it into the next release.

Another thing I would like to improve is the Cache-Control support. I really needed that in a project consuming malloy so I opted for a very simple solution of adding a callable which simply returns the appropriate value for the Cache-Control header field. However, this is currently limited to router::add_file_serving(). At the very least I'm thinking of adding a wrapper/overload which accepts a pure string and creates the callable from that for convenience. My gut feeling tells me that we should have more appropriate infrastructure in place for this sort of thing though.

0x00002a commented 2 years ago

For one we do have #74 / #97 still open. Currently I have a hard time guesstimating whether we should get that into the next release. I fail to get a good metric of how likely it is that this change will lead to unexpected side effects which might need some really proper testing first.

I would say unlikely but I don't want to get burned by that :p, async_initiate is the only risk I can think of as it may have some hidden gotchas. I think the tests should be pretty good at catching issues with lifetime and such, but there is still the bug in gcc that stops the coroutine tests compiling (which I've now reported here). Apart from that though I think its pretty safe, all the breakages are at compile time and code that really needs the future returned from the client::controller methods can pass use_future and get pretty much the same behaviour, the only use case I can think of being stopping running off the end of main. Another thing to consider is that #97 and #102 should probably be put in the same release, as they both break all existing code using the client controller for anything beyond just init.

In regards to the other changes, I agree with your assements but I can't promise to work on anything beyond #97 and #102 for the next few weeks at least