Add ADR on contracts versioning

[Saancreed]

No description provided.

[jakubfijalkowski]

Regarding the open questions:

1. What should the verb/flag to emit compatibility info be called? In the example above, it's compat-info and
   --print-compat-info respectively, but exact naming is TBD.

Just --version? Or --protocol-version if we want to keep --version for purely generator version info

2. In which format should the compatibility info be emitted? It might be JSON or Protobuf, with the former having the
   advantage of being more human-readable while the latter is already natively consumed bythe existing client generator,
   or something else entirely.

IMO, both are fine. I would personally go with Protobuf, since the result of this call should also be strictly defined, and we already have means for that.

3. Do we even need --print-compat-info flag, or perhaps could we get away with always including this info as a part
   of emitted contracts protobuf message?

I don't think we need.

4. Do we need to split version numbers into major/minor/patch?

If going with protobuf, I would just use string, the same way Protobuf uses string in the version for Api type.

5. If we were to keep major and minor version numbers, what kind of changes would warrant bumping major version?

If we go with sem-ver compatibility, breaking the protocol according to Protobuf's "best practices" (if these even exist) would warrant a major bump. Everything else is minor.

6. How should we name and version contracts extensions?

What would be the use case for versioning the extensions? Although I agree with the concept of extensions, I don't think we need them to be versioned. I don't really see any value in versioning them.

When it comes to naming - do we need to decide that right now?

---

Regarding the --print-compat-info - does the version printed depends on the contracts that are generated? I.e. if the contracts don't use DateTime, then the datetime extension won't be reported although the generator itself supports them?

[Saancreed]

Regarding the open questions:

1. What should the verb/flag to emit compatibility info be called? In the example above, it's compat-info and
   --print-compat-info respectively, but exact naming is TBD.

Just --version? Or --protocol-version if we want to keep --version for purely generator version info

I believe --version is reserved for and automatically handled by CommandLineParser. --protocol-version sounds good, but I'll probably need it as a verb (too?), so in the end it'll be something like dotnet tool run dotnet-contracts-generate -- protocol-version.

2. In which format should the compatibility info be emitted? It might be JSON or Protobuf, with the former having the
   advantage of being more human-readable while the latter is already natively consumed bythe existing client generator,
   or something else entirely.

IMO, both are fine. I would personally go with Protobuf, since the result of this call should also be strictly defined, and we already have means for that.

Sure. The humanreadability will suffer a bit but nothing one can't fix by piping the output to protoc.

3. Do we even need --print-compat-info flag, or perhaps could we get away with always including this info as a part
   of emitted contracts protobuf message?

I don't think we need.

Do you mean "we can just include it in the output always" by that? The Export message would have to have this info appended to it as there are no free field numbers before existing fields. Can protobuf handle decoding a message partially if fields before the one we are interested in are mangled in a backward-incompatible way?

I still think there's nonzero value in having this information in context of given contracts input, see my reply to the last question.

4. Do we need to split version numbers into major/minor/patch?

If going with protobuf, I would just use string, the same way Protobuf uses string in the version for Api type.

👍

5. If we were to keep major and minor version numbers, what kind of changes would warrant bumping major version?

If we go with sem-ver compatibility, breaking the protocol according to Protobuf's "best practices" (if these even exist) would warrant a major bump. Everything else is minor.

👍

6. How should we name and version contracts extensions?

What would be the use case for versioning the extensions? Although I agree with the concept of extensions, I don't think we need them to be versioned. I don't really see any value in versioning them.

Okay, imagine that we introduce an extension like DateTime or Decimal, initially at version 1. Then, some extensions might gain additional features in the future, like Decimal starting with support only for .NET's System.Decimal and later gaining support for IEEE 754 Decimal{32,64,128}, warranting bump to 1.1. Or we could decide at some point that we want to change DateTime's serialization format, increasing precision from 1 ms to 1 µs and therefore the number of digits after the dot, which is probably enough of a breaking change to bump extension version to 2.

Is this good enough of a reason?

When it comes to naming - do we need to decide that right now?

Maybe not, but sooner than later would be preferable; support for System.DateTime is likely to become the first one and it's just around the corner.

Regarding the --print-compat-info - does the version printed depends on the contracts that are generated? I.e. if the contracts don't use DateTime, then the datetime extension won't be reported although the generator itself supports them?

Yes, that's the idea. Or, going with the extension example above, if generated contracts contain System.Decimal but not Decimal128, it could report required version of Decimal extension to be 1 instead of 1.1, even though in a vacuum the generator supports up to the latter.

[jakubfijalkowski]

Sure. The humanreadability will suffer a bit but nothing one can't fix by piping the output to protoc.

I think we can provide both (e.g. --protocol-version + --protocol-version-json?). Considering that this tool is either used standalone (in CI), where we don't really need to know the versions, or as part of other tool, then the JSON version is a nice-to-have IMO. Or it can even be non-json, just human readable.

Is this good enough of a reason?

This is perfect reason, but I think this an overkill. Versioning contracts.proto will be hard in itself, versioning extensions will just add to the problem. In the example you provided, we can also go a different route - multiple extensions. One for .NET's decimal, and then we would add another one for the other decimals. We would achieve the same, but I think will be easier to understand and manage.

I also haven't seen that many "extension feature flags" that are versioned - most tools go with just a name . And if version is needed, it can be easily added to the feature name.

Also, I don't expect the "extensions" to stay as extensions for long. It would be too complex for us to manage and won't benefit us that much.

Maybe not, but sooner than later would be preferable; support for System.DateTime is likely to become the first one and it's just around the corner.

I would say - let's go with the flow. I don't think we need fully-fledged extension naming right now, as we only plan to have two extensions, and using datetime and decimal for them would be good enough. :) If we ever find the need for more complex/structured naming, then we'll introduce one.

Yes, that's the idea. Or, going with the extension example above, if generated contracts contain System.Decimal but not Decimal128, it could report required version of Decimal extension to be 1 instead of 1.1, even though in a vacuum the generator supports up to the latter.

I think this (runtime detection) will be mostly unused feature, but this is just a hunch. Let's go with it, provided it won't take too long to develop.

---

To sum up - I would ditch versioning of the extensions. I don't think it will be usable because of the complexity. I would implement the runtime-feature-detection provided it won't be too complex. I also don't think we need any complex extension naming.

[Saancreed]

For the record, here are some remarks I received from @tomaszlaskowskiLC via other channels:

• Versioning extensions seems like an overkill, adding a suffix like -v1.1 should be fine.
• Simplest naming is best – datetime is a good example.
• Runtime detection is a nice feature, but if it would slow down development of this feature, then we should drop it for now.

[github-actions]

Test Results

  2 files  ±0    2 suites  ±0   45s ⏱️ -1s
 85 tests ±0   85 ✅ ±0  0 💤 ±0  0 ❌ ±0 
184 runs  ±0  184 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 12ac880. ± Comparison against base commit f894005.

[jakubfijalkowski]

I think we need (not in this ADR; probably in the docs) to define or at least recommend how the client generators behave if they don't support an extension.

I don't really see how a client generator can (fully) work if they don't support e.g. DateTime extension. The generated contracts will be wrong/unusable, at least in the places where the DateTime is used. Should it just fail? Should it emit a warning? If they emit a warning, what should be emitted in the code? Should we even document that or should we leave it for the clients to decide?

[Saancreed]

For now I'm leaning towards "report a warning, then error out when some unrecognized type is actually encountered" but we'll see.