workflow-execution-service-schemas icon indicating copy to clipboard operation
workflow-execution-service-schemas copied to clipboard

Document versioning

Open david4096 opened this issue 7 years ago • 7 comments

The current versioning scheme does not follow semantic versioning. We should move towards a semantic version to make it clear when there are breaking changes.

david4096 avatar Jul 17 '18 22:07 david4096

@david4096 — are you and @natanlao automating this as part of the build process (e.g., with something like bumpversion? Seems like it might be nice, especially for patches.

jaeddy avatar Nov 26 '18 16:11 jaeddy

We don't really have a clear concept of "patch" vs. "minor" version at the moment, but I think we're probably OK with clarifying breaking changes moving forward. Need to think about how to address this in contributing docs.

jaeddy avatar May 03 '19 00:05 jaeddy

@jaeddy -- I almost feel this request is more of an implementation detail for spec maintenance. It seems manually updating has worked out well enough in the past 2 years -- I'd be curious for your 2 cents on looking back, if you think adding a bumpversion would've helped catch any human errors or simplified the process of versioning?

ruchim avatar Nov 19 '20 04:11 ruchim

@david4096 -- do you feel https://github.com/ga4gh/workflow-execution-service-schemas/blob/develop/openapi/workflow_execution_service.openapi.yaml#L5 gets to the right level of semantic versioning?

ruchim avatar Dec 08 '20 03:12 ruchim

I think this is an issue relevant to all Cloud WS APIs and possibly other WS as well.

Two points from my side in favor of adopting fully SemVer-compliant versioning, one technical, one more "ideological":

  • When implementing WES and other Cloud WS APIs, I occasionally ended up having to check the commit SHA & history to see if I'm still working with the latest version, because that info cannot be parsed from the specs themselves if there's no bump in the version. This is tedious and error-prone and difficult to automate.
  • As a standard-setting organization, I feel the GA4GH would do well in leading by example when it comes to adopting other widely used community standards, such as SemVer.

When it comes to bumping versions, SemVer stipulates the following:

  1. MAJOR version when you make incompatible API changes,
  2. MINOR version when you add functionality in a backwards compatible manner, and
  3. PATCH version when you make backwards compatible bug fixes.

For the Cloud WS APIs at least, I would propose to translate this in the following way:

  1. Increment MAJOR version for breaking changes to the API
  2. Increment MINOR version for backwards-compatible changes to the API (e.g., add a new endpoint or add an optional property to a schema used in an existing endpoint)
  3. Increment PATCH version for any changes that do not alter the behavior of the API (e.g., refactoring OpenAPI document, update descriptions/documentation)

To me this seems pretty much non-ambiguous. Tooling could be used to bump versions automatically, given a keyword in the commit message(s) (adopting Convential Commits may allow us to use existing tooling) associated with a given PR.

uniqueg avatar Jan 26 '21 23:01 uniqueg

Note that adding an optional field in an expected request is backwards compatible. In a response, however, a simple non-optional field may also be considered backwards compatible, if it is assumed that the client is non-strict and ignores additional fields.

I think this assumption is reasonable, because a client that is so strict as to complain about added fields in responses will not be forward-compatible for an API that follows SemVer2 versioning. I bet client implementors will want to avoid an unnecessary exact version lock-in, but rather profit from the semantic versioning scheme.

vinjana avatar Sep 27 '22 11:09 vinjana

There is (now) tooling for classifying breaking and non-breaking changes in OpenAPI: https://bitbucket.org/atlassian/openapi-diff/src/master/

The underlying classification rules are:

Breaking Changes that would make existing consumers incompatible with the API, for example:

  • removing a path
  • removing a method
  • changing a property from optional to required in a request body
  • changing a property from required to optional in a response body

Non Breaking Changes that would not make existing consumers incompatible with the API, for example:

  • adding a path
  • adding a method
  • changing a property from required to optional in a request body
  • changing a property from optional to required in a response body

Unclassified Changes that have been detected by the tool but can't be classified, for example:

  • modifications to X-Properties

We have recently included openapi-diff in the CI for TES: https://bitbucket.org/atlassian/openapi-diff/src/master/

Note that this can only assist in deciding which version to bump, because:

  • The tool does not differentiate between "minor" and "patch" (e.g., typos in descriptions, exploding models) changes
  • It may not currently be possible to specify certain complex requirements in OpenAPI 3.x (or weren't in OpenAPI 2.x and the specs have not yet been updated since migrating)
  • Not all features of the latest OpenAPI version may be supported by this and other tools (e.g., validators, code generators), which might lead to things breaking even for "non-breaking" changes
  • Any other requirements "hidden" in descriptions (of which there are, unfortunately, quite a few across the Cloud APIs)

uniqueg avatar Sep 27 '22 17:09 uniqueg