Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add info about breaking changes to contributing guide #854

Merged
merged 11 commits into from
Oct 26, 2023
14 changes: 13 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,18 @@ move forward.
The AsyncAPI spec, despite describing technical behavior, is intended to be
read by people. Use natural tone and include motivation and examples.

## Breaking Change vs Non Breaking Change

Specifications are not much different than software. Some changes provided in the spec can cause breaking changes for tools that support it. For example, if one of the properties that was `required` becomes `optional`, it is considered a breaking change because some tools might depend on that property and fail if all suddenly it is not provided.
Dule-martins marked this conversation as resolved.
Show resolved Hide resolved

_Non-breaking changes:_
- Adding a property
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be adding an optional property?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, definitely. If you add a required property, it's a breaking change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dalelane makes sense, can you make a code change suggestion for @Dule-martins please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@derberg I just committed all suggestions made here.

Dule-martins marked this conversation as resolved.
Show resolved Hide resolved

_Breaking changes:_
Dule-martins marked this conversation as resolved.
Show resolved Hide resolved
- Making a property required
- Making a property optional
- Removing a property
- Changing the type of a property in any way
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intended to be an exhaustive list or just an example?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from the suggestion made I think it is a list, I can't be sure of how exhaustive it is nor if it is an example.
Maybe @derberg could explain that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is intended as a result of #688

it should be explicit

Dule-martins marked this conversation as resolved.
Show resolved Hide resolved

## RFC Contribution Champions

Expand Down Expand Up @@ -245,4 +257,4 @@ A pull request can be merged if all the following conditions are met:


## References
This document was adapted from the [GraphQL Specification Contribution Guide](https://github.com/graphql/graphql-spec/blob/main/CONTRIBUTING.md).
This document was adapted from the [GraphQL Specification Contribution Guide](https://github.com/graphql/graphql-spec/blob/main/CONTRIBUTING.md).