Skip to content

Cleanup for pull requests and versioning policy. #49

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Aug 24, 2018
Merged

Cleanup for pull requests and versioning policy. #49

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fa6f394
Add pull request template.
lizroth Aug 21, 2018
97a6f8a
Clarifications to versioning policy.
lizroth Aug 23, 2018
e627724
Fixes to RST lists in VERSIONING.
lizroth Aug 23, 2018
366bd1b
Small language consistency tweaks to VERSIONING.
lizroth Aug 23, 2018
874b281
Update VERSIONING.rst
juneb Aug 23, 2018
363a881
Additional minor tweaks/cleanup to VERSIONING.rst.
lizroth Aug 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
*Issue #, if available:*

*Description of changes:*


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
28 changes: 20 additions & 8 deletions VERSIONING.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,30 @@
Versioning Policy
*****************

We will use a three-part X.Y.Z (Major.Minor.Patch) versioning definition with the following meanings.
We use a three-part X.Y.Z (Major.Minor.Patch) versioning definition as follows:

* X (Major) version changes cover changes to the code-base that are expected to break backwards compatibility.
* Y (Minor) version changes cover moderate changes. These include significant (non-breaking) feature additions and might contain changes which break backwards compatability. If there are breaking changes, they will be explicitly stated in the release notes.
* Z (Patch) version changes cover small changes. They will not break backwards compatibility.
* **X (Major)** version changes are significant and expected to break backwards compatibility.
* **Y (Minor)** version changes are moderate changes. These include:

* Significant non-breaking feature additions.
* Potentially backwards incompatible changes. Any such changes will be explicitly stated in the release notes.
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be good to explicitly differentiate these breaking changes from the breaking changes for which we will introduce major changes.

Maybe Less significant but backwards incompatible changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current language, after review/discussion with @juneb is:

Possible backwards-incompatible changes. These changes will be noted and explained in detail in the release notes.

My small pushback is that minor changes are already described as "moderate", less significant than X changes, just above. Reiterating that in the bullets adds grok friction; if it's meaningfully clearer/more explicit I'm cool with that, though.

My goal is to keep this as quick and easy to understand as possible for customers.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. I think when I got into the inner bullet points my brain lost context that we had already stated that they were moderate changes.

* Changes to our package's declared dependency versions.
Copy link
Member

Choose a reason for hiding this comment

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

declared dependencies or dependency versions

Just to be explicit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the old language, there's a few changes since. The newest version is:

Any change to the version of a dependency.

Is that clear enough or would you still prefer changes?

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we do a minor rev on patch changes to dependencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Under this contract, yes.

That seems sensible to me for the following reasons:

  1. Versioning policies differ from dependency to dependency, so we can't make reliable predictions on how our dependencies will change, only on how we'll notify our customers of changes.
  2. Dependency updates tend to happen in clusters, so I expect to see things like 2-3 dependencies having patch rev updates in one of our changes.
  3. Part of the goal here is to be clear, predictable, and as safe as possible. Changing dependencies always carries some risk. Doing a minor version rev every time adds some noise, but provides a reliable, clear bound and helps ensure Z updates are always safe.

Copy link
Member

Choose a reason for hiding this comment

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

Following on from this morning's discussion, if we want to start being more prescriptive about our stated support for max versions as well, then I think what we're wanting to convey here is that we will minor version bump on two events:

  • we change the required minimum version of a dependency
  • we introduce a new dependency

Though in writing that up, I remembered that Maven only supports explicit version statements, not ranges...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we want to start being more prescriptive about our stated support for max versions as well

If we do, then why wouldn't we change the minor version if we change our max supported version of a dependency? That's a change to what we support that can cause integration work for customers, if we need to (for example) pin to an earlier version of a library pending a fix.

All of these cases seem to me like they still fall cleanly under "Any change to the version of a dependency.", which is also language/platform/packaging system-agnostic.

Copy link
Member

Choose a reason for hiding this comment

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

What I worry about there is the different release cultures that different languages have and how that will affect the versioning we advertise. If we assume the dependency releases are rare, then sure, any changes to advertised support for dependency versions should be fine. But if dependency releases are frequent and we want to be explicit about what dependency versions we support, I worry that bumping the minor version every time that advertised support set changes will result in our minor version changes becoming frequent events, and in turn making it more difficult for consumers of our libraries to keep up to date.

I guess this also stems from the very different ways that dependency versions are handled in different languages and distribution systems. For cases like Maven where we explicitly state the version we expect, the relationship is necessarily much different than cases like common PyPI usage where we instead state a closure of versions that we expect to work.

Copy link
Contributor

Choose a reason for hiding this comment

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

No more concerns from me!


* **Z (Patch)** version changes are small changes. These changes will not break backwards compatibility.

* Where possible, we will advise of upcoming breaking changes with warnings in a Z release.

What this means for you
=======================

We definitely recommend always running on the most recent version of our code. This is how we recommend doing so.
We recommend running the most recent version. Here are our suggestions for managing updates:

* Expect X changes to require effort to incorporate.
* Expect Y changes not to require significant effort to incorporate.

* If you have good unit and integration tests, these changes are generally safe to pick up automatically.

* Expect Z changes not to require changes to your code. Z changes are intended to be picked up automatically.

* X changes will likely require dedicated time and work to incorporate into your code-base.
* Y changes are unlikely to require significant (or any) work to incorporate. If you have good unit and integration tests, they can likely be picked up in an automated manner.
* Z changes should not require any changes to your code and can be picked up in an automated manner. (Good unit and integration tests are always recommended.)
* Good unit and integration tests are always recommended.