I would advocate for a much higher level plugin API in the direction of Cripts that encapsulates much of the minutia of hooking into the transaction lifecycle and reduces the rote boilerplate that exists in current plugins. Hopefully something that will cover the vast majority of use cases with fixed-function capabilities. I think it would be fine for mechanisms like those in Cleanup.h to be part of that implementation where we are free to improve on it without impacting the API.
I am not saying we must delete Cleanup.h or even that it is not handy, I'm just not for adopting it as part of the supported TS API at this time. Sent with Proton Mail secure email. ------- Original Message ------- On Tuesday, September 5th, 2023 at 11:37 AM, Walt Karas <wka...@yahooinc.com.INVALID> wrote: > Would it be possible to propose concrete alternatives? We can't just > simply delete Cleanup.h, the xdebug plugin won't compile. > > On Tue, Sep 5, 2023 at 12:27 PM Chris McFarlen ch...@mcfarlen.us wrote: > > > I have read through Cleanup.h and its uses and I agree this is not a style > > we should promote. This code already feels like tech debt in that its > > bridging C and C++ APIs in a bolt-on fashion that I think we will want to > > deprecate shortly. > > > > While certainly RAII and "smart" pointers are important to safety, these > > concerns should be "near to" the structures they operate on and not a > > separate layering of code that is opt-in and incurs additional incidental > > complexity. > > > > For example: > > > > // Deleter and unique pointer for TSIOBufferReader. Care must be taken > > that the reader is deleted before the > > // TSIOBuffer to which it refers is deleted. > > > > In a modern C++ API, we would ensure that TSIOBuffer is automatically > > destroyed only after all TSIOBufferReader instances that reference it are. > > I realize this is not a situation that Cleanup.h created, but in doing > > nothing to address this lifetime issue, you can see how it feels like tech > > debt adding it to the API. > > > > I am also not a fan of using macros to define types and neither is my IDE. > > > > Can we proceed with this in an experimental/internal status for some time > > as James recommends? > > > > Sent with Proton Mail secure email. > > > > ------- Original Message ------- > > On Tuesday, September 5th, 2023 at 10:53 AM, Masakazu Kitajo < > > m4s...@gmail.com> wrote: > > > > > I was not talking about the need to use RAII. I just don't think we > > > should > > > promote Cleanup.h. > > > > > > On Tue, Sep 5, 2023 at 7:28 AM Walt Karas wka...@yahooinc.com.invalid > > > > > > wrote: > > > > > > > It seems that some are not convinced of the need to use RAII. I won' t > > > > get > > > > into that, it's easy to find writeups advocating for it, which are > > > > better > > > > than anything I could write. > > > > > > > > I have no strong feelings about how things are spelled or abbreviated. > > > > > > > > On Sat, Sep 2, 2023 at 11:22 PM James Peach jpe...@apache.org wrote: > > > > > > > > > > On 2 Sep 2023, at 3:44 am, Masakazu Kitajo m4s...@gmail.com wrote: > > > > > > > > > > > > > Its a judgement call, how much to minimize the API. > > > > > > > > > > > > Yes, that's why we send API proposals on the dev list. And I don't > > > > > > think > > > > > > I'm absolutely right. If we, as a community, want to have > > > > > > utilities as > > > > > > TS > > > > > > API, that's what we should do. I'm disappointed that others don't > > > > > > join > > > > > > this > > > > > > discussion, but if they don't mind making Cleanup.h TS API, that > > > > > > means > > > > > > we > > > > > > have many potential maintainers. I guess I could just abide and > > > > > > hope > > > > > > you > > > > > > and they maintain it. > > > > > > > > > > > > Is providing Cleanup.h as TS API the one and only way? I don't > > > > > > understand > > > > > > why not promoting Cleanup.h means inevitable resource leak. Plugin > > > > > > developers can take care of it in their own way, right? One could > > > > > > copy > > > > > > Cleanup.h anytime if they want. > > > > > > > > > > It looks like there’s only one internal plugin using this, so it’s > > > > > not > > > > > self-evidently essential. How about adopting it internally before > > > > > making > > > > > it > > > > > public API? > > > > > > > > > > Also, could we change it to spell `UniqPtr` correctly? > > > > > > > > > > J
