Versioning

[QuintinWillison]

For service version 2 (a.k.a. '(wire) protocol v2') we are also moving the specification version from 1.2 to 2.0, however the implications of that specification update for client libraries (what's in the target integration/protocol-2.0 branch already) mean that they can adopt this new service version as a non-breaking/compatible change.

I'm keen to use this, our first clearly available opportunity, to better define the separation between specification version and service version. I hope reviewers will find this adequately covered and clearly defined in the change proposed in this pull request. 🤞

The following preview links for this pull request may be helpful to reviewers, in order to shortcut to the rendered output for the key changes being made here:

Location	Link	Longevity
Deployment for this PR (rendered textile)	Features: Client and Service Versions	Will remain after this PR merges.
GitHub UI view of the branch underlying this PR	README.md: Versions	Will disappear after this PR merges, as we automatically delete feature branches at that point.
GitHub UI view of the branch underlying this PR	CONTRIBUTING.md: Features Spec Points	Will disappear after this PR merges, as we automatically delete feature branches at that point.

This pull request is my second attempt at this, after I abandoned #105 (my first attempt) some time back. I have endeavoured to reflect and act upon the feedback received in that initial attempt (from @SimonWoolf, @lmars and @lawrence-forooghian) here.

Closes #6.

[owenpearson]

LGTM! Thanks for putting those links in the description :)

A couple of notes:

• Rather than including the noise of bygone spec points in the specification itself at the cost of readability, might it be worth making a script to detect resurrected spec point IDs in CI (similar to find_duplicate_spec_points)?
• I looked through some examples of recent spec changes and I think there are probably some edge cases where it's not clear to me how specification version would be incremented. Clarifications to the spec like Require agents specified by ClientOptions#agents to have a version #112 Spec: add G4a, clarifying that protocol version should be treated literally #97 would only necessitate a change to SDK implementations in some cases so would you say those are major or minor?

[QuintinWillison]

Thanks for your approval comment, @owenpearson. Your observations touch on areas that have definitely been swimming around my head while incubating this work! 😅 Here's my thoughts...

Noise of bygone spec points

These are not the prettiest things to wade past when reading the features spec, so I get your point. For now, however, I would prefer to keep them in:

• They provide helpful and immediate 'bread crumbs', especially when deep linked to, in particular when those deep links go to the rendered output from HEAD from the main (default) branch... for example:
  • to reassure the visitor that they've not clicked a bad link
  • for Removal to ensure the visitor understands that the place they linked from might be referencing an older or wrong version of the spec
  • for Replacement to help the visitor quickly navigate to their replacement(s)
• We can work on a solution for solving this down the line when we get more annoyed with them taking up space (the problem is not big today) - e.g. perhaps format them differently; or even 'fold them away' by default, using CSS+JS from the rendered document
• They'll be easy to find, given conformed form, if/when we come to deciding to remove them or apply some special processing to them at point of render

Edge cases for specification version increments

Clarification of edge cases is definitely important, but given the reviewer count on this PR then I would ideally like to see reviews come in based on this current scope (warts and all) and then iterate with a new PR later on... either proactively covering past edge cases, and/or reactively covering future edge cases as we encounter them.

As we've discussed elsewhere, this is still an improvement so worth merging - on the assumption there's not feedback from others to raise concerns relating to something being blatantly wrong or unworkable. 🤞

[lawrence-forooghian]

This looks great, and thank you for making it easy to review! I have a couple of overall questions…

Why do we have a Specification version number?

I'd be interested to know what kind of use cases you have in mind for the Specification version number, and who it might be used by.

The only thing I could speculate was that you were thinking that perhaps our client library SDKs might (in some manner) wish to claim to conform to a given Specification version? If so, do we need a version number for that? Given that the feature spec points are (now) immutable, we could define a client library’s feature spec compatibility in terms of which feature spec points it implements.

Release process

CONTRIBUTING.md currently says that the release process will be documented as part of working on #6 (which this PR closes). I can't see anything about the release process in this PR, though.

I think that knowing how the release process works might help answer some questions that this raised for me about how the future process of developing the features spec would work.

For example, a feature spec writer who is removing a feature spec point is instructed to write It was valid up to and including specification version @X.Y@. It's not very clear to me where I would get this X.Y version from - it depends on whether I'm working on a branch on which the specification version hasn't been bumped yet (in which case I get the version from the meta.yaml file on my branch), or on a branch on which the specification version has already been bumped (in which case I'm not sure where I'd get the version from), which ultimately depends on the branching and release strategy for the repo.

Similarly, if a writer makes a change to the feature spec that necessitates a major / minor / patch version change, do they make that version bump in their PR or does that happen at the end of some sort of release process?

[QuintinWillison]

Thanks for your review input, @lawrence-forooghian. In response to your overarching commentary:

• Why do we have a Specification version number?: I can see where you are coming from here and I'm happy to try and explain my thoughts, though I would prefer to defer any further refinement or explanatory commentary additions to a later PR so we can get this merged...
  • You're right that (now immutable) spec point references alone should be enough for SDK implementations to reference (informal comments, formal annotations, etc..), therefore no need for SDKs to reference the specification version as primary anchor.
  • The primary content owned (as source) and evolved in this repository is the specification, which includes the content of all files in the textile folder. The 'features spec' document (features.textile) is a complex document which covers a wide scope of concerns from app/user SDK API all the way through to service protocol details, however it's only one of the files in that folder, expected to continue to evolve in-step with them (especially protocol.textile). I feel this complexity transcends features spec points alone, therefore deserving of a higher level of granularity in tracking (being the specification version).
  • I've explained in my new README.md section on "Versions: Specification Version" the scenarios where I see major vs minor vs patch bumps to this SemVer value. I think it's useful to be able to discuss and reason about the specification in semantic versioning terms like this, bringing a collective understanding of "the whole" where a focus on specific features spec points is too detailed. There's an element of gut feeling here, a pinch of me connecting dots from elsewhere that might not be so obvious to others, plus I recognise that this ties in with the release process which I still have to document (see next point).
• Release Process: I've created a new issue to defer this to a later PR, fixing the referencing issue number accordingly in 49722b3

Hopefully my responses to your individual, specific comments on this PR are enough to get those conversations resolved. I also hope that this response is enough to gain your overall approval for this work. 🙏

[SimonWoolf]

Generally all sounds fine to me.

One thing I might question is the name "Service version". That sounds like it should mean 'the version of the Ably service', but as used here it doesn't (and as you mention, a given version of the ably service supports multiple protocol versions). IMO 'protocol version' is a more natural name, as in eg "service version release-20221205.46-012252fc3 supports protocol versions 0.8 through 2"

I'm also not sure I'm convinced that protocol version is metadata (in meta.yaml) rather than data. The spec version is metadata, sure, but the protocol version -- when that changes, that's a concrete change that needs to be made in every SDK (alongside its sibling changes), to be tracked and markable as done; doesn't that deserve the 'spec item replacement' treatment?

[QuintinWillison]

Thanks, @SimonWoolf.

• Renaming "service version" to "protocol version": I agree. Your reasoning is very sound on that. I'm going to drop this pull request into Draft state again while I address that. I will then re-request reviews from those who have already approved as this change is pretty fundamental. 😅
• Location of Protocol Version Definition: I can see where you're coming from, however I am seeing the state of this repository evolving as an atomic whole, with my intention being to only have these version constants defined in a single location. I feel that my new features spec point CSV2 covers this well, with the rendered output making sense. I fear we could be at risk of splitting hairs and/or going around in circles (given this is my second attempt at this). I feel that, from an SDK Developers perspective, the guidance will be clear enough. If this choice starts to cause issues down the line then we can rethink then.

[lmars]

LGTM 🙌