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

Test emulated changes against the API #104

Closed jonaslagoni closed 3 years ago

jonaslagoni commented 3 years ago

After https://github.com/asyncapi/shape-up-process/issues/93 we should test the emulated changes against the API to ensure that it do not break it.

jonaslagoni commented 3 years ago

Copy pasted from https://github.com/asyncapi/parser-js/pull/263#issuecomment-806996059

Emulated breaking changes

We needed to test the intents based on different changes from https://github.com/asyncapi/shape-up-process/issues/93

3.0.0-fake

Assumptions:

Intents:

3.0.0-fake2

Because this makes it possible to define multiple schema formats per message, it is no longer a 1:1 (message:schemaFormat) but a 1:* and breaks the return values

3.0.0-fake3

Because this makes it possible to define multiple operations per channel, it is no longer 1:1 (channel:operation) but a 1:* and breaks the return values

3.0.0-fake4

Because this makes it possible to define multiple messages under a channel, and those messages can contain operations we no longer have a 1:1 (channel:operation) but a 1:* and breaks the return values

The only apparent solution is to return an array of values, always, to be safe. But this is affecting the toolings using the parser making it more difficult...

jonaslagoni commented 3 years ago

We need to take a design decision in regard to the breaking changes and how we want to guard the API.

  1. Always return arrays This makes it somewhat harder for developers to use. We would have to change Message.schemaFormat() : string to return string[] instead.

  2. Accept it will be breaking changes when the cardinality of return values changes We simply accept that any cardinality changes to the return values will create a breaking change.

derberg commented 3 years ago

cardinality changes

sorry but I have no idea what cardinality means in this context, you mean function return type?

jonaslagoni commented 3 years ago

cardinality changes

sorry but I have no idea what cardinality means in this context, you mean function return type?

Yes, updated the comment :)

fmvilas commented 3 years ago

Accept it will be breaking changes when the cardinality of return values changes We simply accept that any cardinality changes to the return values will create a breaking change.

I'd go for this solution. In case it happens, we can always default to returning the first value of the array and add another method that will return the array for those who want to upgrade to the new functionality. For instance:

Message.schemaFormat() : string would remain the same but we add Message.schemaFormats() : string[]. At some point, we remove the first method.

Might not be perfect but it's an easy-to-handle situation.

jonaslagoni commented 3 years ago

Copy pasted from https://github.com/asyncapi/parser-js/pull/263#issuecomment-816652619

With a new iteration for the intent API another round of checking breaking changes.

Emulated breaking changes

It seems like we solved the problems of the breaking changes we had in the last suggestion 👏

3.0.0-fake

Since we already return an array of operations, this will not break the parser API.

3.0.0-fake2

Does not break the parser API since we can add support function such as getAllSchemaFormats to the standard getSchemaFormat so we deprecate the old function as time goes on.

3.0.0-fake3

Still works since we have a bidirectional flow of channels, messages, and operations already.

3.0.0-fake4

Still works since we have a bidirectional flow of channels, messages, and operations already.

jonaslagoni commented 3 years ago

I think we have a pretty good understanding of what will force breaking changes moving forward. Think we can close this issue, what do you think @smoya ?

smoya commented 3 years ago

+1 @jonaslagoni