Git layout for v1.15.0

[freeekanayaka]

It seems that the v1.15.0 tag has been put on a separate release-v1.15.0 branch, instead of putting it on master like we usually do.

Since afaics the release-v1.15.0 branch has the same content of master, minus the new dqlite_server_xxx APIs, I presume the intention was to leave out the changes from #514 onwards.

However this effectively is a fork of master, with a few possibly unfortunate consequences:

• the v1.15.0 tag does not belong to master's history, so when looking at master's history in the future it won't be possible to easily tell if a change belongs to v1.15.0 or to the next version (say v1.16.0, assuming that the next release tag will be placed on master)
• the current history of release-v1.15.0 seems a rebase of master, and does not contain the merge commits, so when looking at release-v1.15.0's history it's not possible to easily tell in which PR a commit is contained
• it's not possible to easily see what common changes/commit release-v1.15.0 and master have (needs to be compared manually)
• since the v1.15.0 tag is placed on the release-v1.15.0 branch, we'll have to keep the release-v1.15.0 branch around indefinitely

I think it'd be fine to simply include the recent dqlite_server_xxx code in the v1.15.0 release, cutting v1.15.0 from master as usual.

If desired, we could introduce a DQLITE_EXPERIMENTAL, similar to SQLITE_EXPERIMENTAL, see the docs and the code.

If there are changes that we don't want to include in the next release,y the probably shouldn't be committed to master in the first place (but I don't think we need to do that, since we merge only working changes that don't make master "unreleasable").

[cole-miller]

I agree that the side effects you list are unfortunate. In the future I'll probably approach this problem by putting the incomplete API surface behind an undocumented #ifdef DQLITE_INCOMPLETE.

[freeekanayaka]

I agree that the side effects you list are unfortunate. In the future I'll probably approach this problem by putting the incomplete API surface behind an undocumented #ifdef DQLITE_INCOMPLETE.

Is there a specific reason you don't want the new APIs to be part of this v1.15.0 release?

It seems to me that they are complete and could already be used, and actually they are what new consumers should use, since they offer better functionality/signatures than the dqlite_node_xxx, which we should already deprecate . These new APIs don't require a client or anything like that.

[cole-miller]

I don't have a specific reason other than giving the new interfaces the maximum time to "cook" before making them accessible in an official release. My upcoming implementation of the remaining parts of the C client API will involve some changes to dqlite_server, and there are some kinds of testing that I would like to do using the full C client API that I suspect will shake out more bugs in the already-merged code.

[freeekanayaka]

Possible bugs are fine, they are possible as in any release, so they shouldn't be a blocker.

Are you envisioning changes to the dqlite_server_xxx APIs or changes to the implementation? The latter case is also fine and normal, not blocking in any way.

I would expect no change to the existing dqlite_server_xxx APIs (adding new APIs in the future is always ok).

I'd propose to just add a no-op DQLITE_EXPERIMENTAL "tag" (or simply prefix the docstring with the word "Experimental"), in the worst case that small tweaks to the signatures are needed that we're not seeing now.

That would leave us more time before we declare those interfaces as final and stable, possibly taking in user input after the release of the experimental version. Having a flag-day switch like #ifdef DQLITE_INCOMPLETE would mean that we'd commit to get them perfect with no chance of small tweaks (we'd also have to multiply the test matrix to run with and without DQLITE_INCOMPLETE).

I'm mostly concerned by having a release-v1.15.0 branch that is an oddball which we have to keep around indefinitely, and by not having the v1.15.0 not be part of the master branch like all other release tags.

Would it be ok for you to mark these APIs as experimental and cut a regular v1.15.0 release from master?

I think it'd be fine to delete the current v1.15.0 tag since it's so recent, and there won't be actual changes for users.

[cole-miller]

I'm not comfortable moving a published tag, but I wouldn't mind introducing DQLITE_EXPERIMENTAL for functions that are implemented but not yet ready to be stabilized. I don't think that approach is right for functions that haven't even been implemented yet, as much of the client API (the non-dqlite_server parts) hasn't. I guess it may have been a mistake to merge the changes to dqlite.h without corresponding sane implementations, although it made sense to me at the time.

[freeekanayaka]

I'm not comfortable moving a published tag

I understand, but what are we going to do with the next release though (say v1.16.0)? It will not have v1.15.0 as git parent? Will we need to keep the release-v1.15.0 branch around indefinitely? It seems odd to me.

Retagging seems still fine for now, so we keep commit/release history in order long-term. We're not going breaking code or anything like that. Just my 2c.

but I wouldn't mind introducing DQLITE_EXPERIMENTAL for functions that are implemented but not yet ready to be stabilized. I don't think that approach is right for functions that haven't even been implemented yet, as much of the client API (the non-dqlite_server parts) hasn't.
I guess it may have been a mistake to merge the changes to dqlite.h without corresponding sane implementations, although it made sense to me at the time.

Ok, we could use DQLITE_INCOMPLETE for those, or something equivalent. In practice, I doubt we will have users complaining about that either way, so although we might not do it again in the future, most probably nobody is going to be mad at that in the present, whether we use DQLITE_INCOMPLETE or not.

[freeekanayaka]

We can also remove the client functions temporarily from master, and re-add them little by little as they get implemented.

[cole-miller]

How about this for a compromise:

• I will open a PR to remove the unimplemented C client functions from dqlite.h on master, and mark the implemented ones as DQLITE_EXPERIMENTAL.
• After that merges, I will tag v1.15.1 at HEAD. This will be functionally the same as v1.15.0, but will be a more "normal" release that can be easily compared to v1.14.0 (and to v1.16.0 when we get around to that).
• v1.15.0 will stay where it is.