search

asyncapi / shape-up-process

This repo contains pitches and the current cycle bets. More info about the Shape Up process: https://basecamp.com/shapeup
https://shapeup.asyncapi.io
11 stars 8 forks source link

Find a way to keep a Parser(s) particular release to support, at least, one particular Spec version. #95

Closed smoya closed 3 years ago

smoya commented 3 years ago

Currently, both the spec and the Parser(s) follow semver and they keep releasing new versions independently. There is no compatibility matrix between the tool and the spec right now.

As new spec majors will bring breaking changes, the Parser(s) will need to support them.

We should find a way to ensure a particular release of the Parser(s) is compatible with, at least, one particular spec version and show that to the user in an intuitive fashion. Meaning the user should know which spec versions are supported by a particular version of the Parser(s) at any moment.

There are several mechanisms to ensure that. The goal of this issue is to get an idea of the pros and cons and finally make a decision to use one mechanism or another.

Things to consider:

Some ideas (not all):

1. As it is now. Keep a fully independent release cycle.

PROS

CONS

2. To map release cycles.

Meaning Parser(s) v3.0.0 is compatible with the version v3.0.0 of the Spec.

PROS

CONS

3. To only map majors.

Meaning Parser(s) v3.* is compatible with the highest v3.* version of the Spec.

PROS

CONS

4. To add the spec version as semver pre-release metadata

Meaning Parser(s) release versions will be a composition of <spec-version>-spec.3.0.0 or similar.

PROS

CONS

asyncapi-bot commented 3 years ago

Hey! You've labeled this issue as a Scope. Remember you can use the following command to inform about its progress:

/progress <percentage> [message]

or

/progress <percentage>

A mutiline message.
It supports **Markdown**.
Example:
/progress 40 We're still figuring out how to implement this. We have an idea but it is not yet confirmed it will work.
/progress 50

A few notes:

* We got this figured out :tada:
* We're going to use [this library](#link-to-website) to avoid losing time implementing this algorithm.
* We decided to go for the quickest solution and will improve it if we got time at the end of the cycle.

:weight_lifting_woman: See the progress on the Shape Up Dashboard.

asyncapi-bot commented 3 years ago

Hey! You've labeled this issue as a Scope. Remember you can use the following command to inform about its progress:

/progress <percentage> [message]

or

/progress <percentage>

A mutiline message.
It supports **Markdown**.
Example:
/progress 40 We're still figuring out how to implement this. We have an idea but it is not yet confirmed it will work.
/progress 50

A few notes:

* We got this figured out :tada:
* We're going to use [this library](#link-to-website) to avoid losing time implementing this algorithm.
* We decided to go for the quickest solution and will improve it if we got time at the end of the cycle.

:weight_lifting_woman: See the progress on the Shape Up Dashboard.

jonaslagoni commented 3 years ago

I think we should go with option 1.

The reason behind this is because if we want to use intents together with SemVer we don't really have any other choice. Why? Because when people add the parser as a dependency for tools they would most likely do something like ^1.4.6, meaning it will use any newer versions if available (as long as it is not a new major version). We break this pattern by choosing any of the other options.

Furthermore, with intents, we are supposed to make a minor version change when a major version of the spec comes out (if possible of course). The only compatibility matrix I see we need is to add what version of the spec the parser supports up to. i.e. if the parser is at v1.x it will support all versions up to whenever we have to break the parser. If that happens when the spec is at v3 the parser would get the version v2 and in the readme for that say: Supports up to version 3 of the spec. For v1 of the parser, it would still say: Supports up to version 2 of the spec.

Or am I missing some specific perspective?

derberg commented 3 years ago

@smoya option 1 It is not yolo like, it is just how it should work in my opinion, you never know what will happen to parser, and when we will need to bump major as we did improve something that is breaking. Like for example we have issue to improve a lot error handling and this will cause major release, independently from the spec version.

We just need clear statement in the readme that anyway will always be visible in the npm too, so you know what version of the spec current version of the parser supports. And as a reference we should have a table that maps spec to parser version. Easy 😄

Supports up to version 3 of the spec

you mean once we have intent API ? or in general. We should just have a table:

assumptions: we release major with intent API before spec v3 and we also release improved error handling before v4 after v3

spec parser
v2.0.0 v1.x, v2.x
v3.0.0 v2.x, v3.x
v4.0.0 v3.x
jonaslagoni commented 3 years ago

How much work do we plan on putting into keeping older versions of the parser up to date with the spec? 🤔

derberg commented 3 years ago

in my opinion, once we have a new version of the spec, we focus only on this in the parser and the community should use converters to migrate or just use older version of the parser for older specs.

jonaslagoni commented 3 years ago

I suggest we use the following way of matching versions of spec and parser(s).

There can be 3 different scenarios for what will happen when a new spec version appear:

  1. New major spec version requires a major version change in the parser since we cannot remap it internally.

Example: Spec changes v1.0.0 -> v2.0.0, which does require a new major version change in the parser v1.0.0 -> v2.0.0

  1. New major spec version does not require major version change in the parser, meaning we can remap it.

Example: Spec changes v1.0.0 -> v2.0.0, which does not require a new major version change in the parser v1.0.0 -> v1.1.0

  1. New minor/fix spec version should never require a new major version change in the parser only a minor version change.

Example: Spec changes v1.0.0 -> v1.1.0, which only require a minor version change in the parser v1.0.0 -> v1.1.0

Besides these scenarios, it could happen that the parser would require a re-write/restructure independently from the spec which of course should be possible.

I suggest we do NOT maintain a compatibility matrix for specific minor versions of the parser. Because anyone should be able to update the dependency if a new minor version is released. Rendering that information irrelevant.

I suggest that we, therefore, maintain the following compatibility matrix in each parser repository:

Specification version Parser version Parser status
v6.x.x and all previous versions v3.x Actively maintained.
v5.x.x and all previous versions v2.x Limited maintained, see migration guideline.
v2.x.x and all previous versions v1.x Limited maintained, see migration guideline.

Actively maintained meaning we actively maintain this version and potentially make it compliant with newer versions of the spec. Limited maintained means we do not actively update this version to be compliant with the new spec versions, but we do fix any potential bugs.

Thoughts?

derberg commented 3 years ago

Limited maintained

you suggest we support older versions for bug fixes. This means we need feature branches, complicate workflows of backporting fixes, cherry peaking commits. My option is, no support, see migration guide, this is why we invest in better resilient API and coverters

v6.x.x and all previous versions

another complication, support latest and all the previous in one code base....maintenance will be complicated

as for the releasing scenarios you described -> yes, so basically we do semver without looking at the spec. In other words no matter if spec has braking changes or not, if parser can avoid them, that is fine

smoya commented 3 years ago

My option is, no support, see migration guide, this is why we invest in better resilient API and converters

That could be like the first option to go. Then we can evaluate, in the future, if we need to support old versions. I think it seems reasonable. WDYT @jonaslagoni ?

jonaslagoni commented 3 years ago

you suggest we support older versions for bug fixes. This means we need feature branches, complicate workflows of backporting fixes, cherry peaking commits. My option is, no support, see migration guide, this is why we invest in better resilient API and coverters

Fair, I can get behind this.

another complication, support latest and all the previous in one code base....maintenance will be complicated

Hmm, we should try though, since this is the core of the implementation and should work.

That being said, I could see that eventually, we wanted to exclude earlier versions, but that can be addressed if that time comes. It might even happen that some previous versions will not be possible for some weird reason.

So maybe something like this?

Specification version Parser version Parser status
v6.x.x down to v2.x.x v3.x Actively maintained.
v5.x.x and all previous versions v2.x Not maintained, see migration guideline.
v2.x.x and all previous versions v1.x Not maintained, see migration guideline.
derberg commented 3 years ago

I think this issue should be moved to parser repo as it concerns parser directly and release process for it than the current bet.

I say we can support older spec versions but only as a side effect...you know what I mean? That we do not have any logic that identifies the spec used by the user and therefore run different parsing logic.

I'm just not sure this is the right moment for this discussion. Shouldn't we get first major release of intend API and then see what we can do with spec support?

jonaslagoni commented 3 years ago

I think this issue should be moved to parser repo as it concerns parser directly and release process for it than the current bet.

@derberg I would disagree, this revolves around all the parsers (primary JS parser, but also for GO). So it is a general discussion rather than for a specific repository 🙂

I say we can support older spec versions but only as a side effect...you know what I mean? That we do not have any logic that identifies the spec used by the user and therefore run different parsing logic.

@derberg Hmm... I see different approaches here which kinda depends on the requirement for the solution and where we put the responsibility. I would say that it should not be a side effect but rather a feature (if possible) since this will provide immense value to the end-user. We cannot assume that they will migrate their AsyncAPI documents and not forcing this feature creates frustration for the end-user, which should be avoidable as much as possible.

However, while that is great and all, we still have to remember that each feature request to the spec is gonna need to change the parser for it to be accepted. This means that maintainability has to be a priority, however, I still think we can implement a solution that can be maintainable while still providing this feature.

What do you think? cc @smoya @derberg

derberg commented 3 years ago

I would disagree, this revolves around all the parsers (primary JS parser, but also for GO). So it is a general discussion rather than for a specific repository

🤷🏼 who from the community will read this issue? shape up is just how we run work in an organized way. Not many people come here and not many people come to Go parser repo. While JS Parser and around 100 visitors every day. This is definitely not a place for such an important discussion. We are discussing here a pretty critical topic.

For me, important is to always support latest, and give tools that make it super easy to support the latest. IMHO currently the scale of active contributors is not large enough to be able to support multiple versions atm. Even the current parser is not fully covering the spec 😞

jonaslagoni commented 3 years ago

🤷🏼 who from the community will read this issue? shape up is just how we run work in an organized way. Not many people come here and not many people come to Go parser repo. While JS Parser and around 100 visitors every day. This is definitely not a place for such an important discussion. We are discussing here a pretty critical topic.

Good point 🤔 Alright I agree to move the issue. We can always create a follow-up issue in the Go parser referring to the discussion. What do you think @smoya ?

smoya commented 3 years ago
  1. I agree we do not have to provide a library backward compatible yet, that's the reason for a little detail you can find on the title of the issue: at least,. At least support one version.
  2. I still think we, at some point (not needed to be now), would need to provide a compatibility matrix on each Parser. Integration tests will be a must for ensuring the matrix table is right. Supporting "latest" is not a valid answer to the user since changes introduced in the spec may come, or not, with changes in the Parser (remember we will have several implementations: JS, Go, Rust, Visual Basic 6, Cobol, Delphi...!)
smoya commented 3 years ago

/progress 65 We wrote down some options to consider. A discussion has been started around and we are trying to reach an agreement.

jonaslagoni commented 3 years ago

Sorry in advance for the long response 😅

Okay, let's clean the slate, and start with the basics. What are we trying to solve:

  1. We need a way to define which parsers support which spec versions from a global perspective.
  2. We need a way to define, in each parser repository which spec versions are supported in what parser versions.

These are the assumptions about the parser setup I made to move forward (remember they are assumptions and can change, however, I think we can use the same solution regardless):

  1. The proposed compatibility matrix will be manually maintained for now. There are ways to automate this with CI but kinda out of scope for now.
  2. Only the newest version of the intent API will be actively maintained.
  3. The intent API will maintain its own release cycle and version since it can be skewed from the spec (handle breaking changes internally).
  4. Each parser will maintain its own release cycle and version.
  5. All individual parsers are following the intent API. Even though that the individual parsers maintain an individual release cycle, changes to the intent API will force the individual parsers to update as well.
    • Say we release a new spec version and we are forced to create a breaking change and force a major version change for the intent API (say from version 2 -> 3) the individual parsers are then forced to make a major version change as well (say JS parser go from version 1 -> 2).
  6. We do not explicitly care about how parsers ensure that they support those specific versions. I think that belongs in another issue as well.
  7. From the start the intent API will not explicitly provide backward compatibility with older specs, it can happen naturally as @derberg pointed out. This issue is not about defining whether the parsers should be backward compatible, so let's keep that discussion to a minimum. Might need a new issue for this?

We will create a follow-up issue in the new parser-api repository since this is not about parser-js. Remember we are just trying to come up with a suggestion on how to specify this. @derberg if you think it makes sense to create the issue in the parser-js repo for a broader audience, I have no objects to that.

I suggest we use SemVer in the compatibility matrix. In the examples, I will provide how to keep a compatibility matrix in a global perspective as well as locally in each parser repo, if that is desired. In the examples I will be using spec versions ranging from v2.0.0 - v6.1.0, to show how it might look in the future. I am gonna include 2 parsers (JS and Go parser) to demonstrate how that should look like. All versions are purely fictional so don't make correlations to any current versions.

Compatibility matrix in a global perspective

This is gonna be located in the new parser-api repository. Specification version Intent API version JS parser Go parser Parser status
6.1.0 ^2.1.0 ^3.1.0 ^1.1.0 Actively maintained.
6.0.0 ^2.0.0 ^3.0.0 ^1.0.0 Actively maintained.
5.0.0 ^1.3.0 ^2.11.0 Not supported Not maintained, see migration guideline for specifiction.
4.0.0 ^1.2.0 ^2.8.0 Not supported Not maintained, see migration guideline for specifiction.
3.0.0 ^1.1.0 ^2.5.0 Not supported Not maintained, see migration guideline for specifiction.
2.0.0 ^1.0.0 ^2.3.0 Not supported Not maintained, see migration guideline for specifiction.

Once we have more parsers than the matrix can display, we can reevaluate how we want to change it. It could be as simple as keeping a matrix per parser. But I feel like this is an issue for later.

Compatibility matrix for local parsers

I would suggest that we follow @smoya suggestion and do not provide a matrix for each repository for now and simply refer to the global matrix in the parser-api repository.

However, if we wanted to have one in each repository, I would suggest it would look something like the following.

For JS parser

Specification version Intent API version JS parser Parser status
6.1.0 ^2.1.0 ^3.1.0 Actively maintained.
6.0.0 ^2.0.0 ^3.0.0 Actively maintained.
5.0.0 ^1.3.0 ^2.11.0 Not maintained, see migration guideline for specification.
4.0.0 ^1.2.0 ^2.8.0 Not maintained, see migration guideline for specification.
3.0.0 ^1.1.0 ^2.5.0 Not maintained, see migration guideline for specification.
2.0.0 ^1.0.0 ^2.3.0 Not maintained, see migration guideline for specification.

For Go parser

Specification version Intent API version Go parser Parser status
6.1.0 ^2.1.0 ^1.1.0 Actively maintained.
6.0.0 ^2.0.0 ^1.0.0 Actively maintained.
<6.0.0 <1.3.0 Not supported

Summary

If you can answer the following question relatively easily I would assume this is okay to use

  1. Can you quickly see which specific parser version support v6.0.0 of the spec? and which intent API version is provided for that?
  2. How many spec versions did the intent API version ^1.3.0 support?
  3. What spec version does the Go parser support?

Some further questions that I had while writing this down:

  1. We should be able to keep the migration guide in the global parser-api repository since the migration is the same regardless of for which parser.
  2. I suggested we simply refer to the global matrix in each parser repo for now to reduce maintenance. So we have a single source of truth, what do you think?

What do you think? Am I missing some perspective somewhere? 🤔

smoya commented 3 years ago

@jonaslagoni your comment here is superb. I wouldn't explain such topic clearly as you did, so thanks for making this happen.

After giving a (more) deep thought on it, I think it would make sense if each repository maintains their compatibility matrix rather than globally in the https://github.com/asyncapi/parser-api. Otherwise, every new release on each parser implementation will carry a change on the https://github.com/asyncapi/parser-api repository, which IMHO is a bit tedious and can lead to errors. However, no strong opinion. I agree on all the points you made.

jonaslagoni commented 3 years ago

After giving a (more) deep thought on it, I think it would make sense if each repository maintains their compatibility matrix rather than globally in the https://github.com/asyncapi/parser-api.

Good point, I would be open to that idea as well, easier from the maintainer's point of view.

smoya commented 3 years ago

👍 on my side to close this issue. New issues will come up on each parser implementation repository once they migrate to the new API. cc @jonaslagoni @derberg

smoya commented 3 years ago

As concluded, each parser repository will maintain their compatibility matrix. Mentioned in https://github.com/asyncapi/parser-api/pull/1/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R26.

smoya commented 3 years ago

/progress 100 conclusions reached. As mentioned in https://github.com/asyncapi/parser-api/pull/1/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R26, each parser repository will maintain its compatibility matrix.