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.

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

Document versioning and deprecation policy for major versions #11205

Closed
wants to merge 9 commits into from
Closed

Document versioning and deprecation policy for major versions #11205

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7e0cb39
Document versioning and deprecation policy for major versions
mtreinish Nov 6, 2023
c8bed1f
Fix heading consistency
mtreinish Nov 6, 2023
c34d724
Update docs/deprecation_policy.rst
mtreinish Nov 7, 2023
7fc4dfc
Add paragraph on public API surface
mtreinish Nov 7, 2023
434c788
Apply suggestions from code review
mtreinish Nov 7, 2023
067247c
Update docs/deprecation_policy.rst
mtreinish Nov 7, 2023
c46454d
Update text
mtreinish Nov 10, 2023
72012b9
Merge remote-tracking branch 'origin/main' into 1.0-policies
mtreinish Nov 10, 2023
b38ce5b
Update docs/deprecation_policy.rst
mtreinish Nov 15, 2023
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
106 changes: 101 additions & 5 deletions docs/deprecation_policy.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,96 @@
#############################
Versioning and Support Policy
#############################

Qiskit version numbers follow `Sematinc Versioning <https://semver.org/>`__.
mtreinish marked this conversation as resolved.
Show resolved Hide resolved
The version number is comprised of 3 primary components, the major, minor, and
patch versions. For a version number ``X.Y.Z`` where ``X`` is the major version,
``Y`` is the minor version, and ``Z`` is the patch version.

Breaking API changes are reserved for major version releases. The **minimum**
period between major version releases is one year. Minor versions will be
periodically (currently every three months) published for the current major
version which adds new features and bug fixes. For the most recent minor version
mtreinish marked this conversation as resolved.
Show resolved Hide resolved
there will also be new patch versions published as bugs are identified and fixed
on that release series.

For the purposes of semantic versioning, the Qiskit public API is considered
any documented module, class, function, or method that is not marked as private
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the use of "documented" here, but I am not sure it agrees with the text in the "Deprecation Policy" section below:

Beware that users will often be using functions, classes and methods that we,
the Qiskit developers, may consider internal or not widely used.  Do not make
assumptions that "this is buried, so nobody will be using it"; if it is public,
it is subject to the policy.

and

never assume that a function that isn't explicitly internal isn't in use

If something is not documented but does not start with a _, can it be removed? Should "public" in the quoted section above be "publicly documented?"

Copy link
Member Author

Choose a reason for hiding this comment

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

I should update the deprecation policy section below I think. There was an ambiguity in expectations for some people around public functions that are in weird modules. For example something like qiskit.transpiler.passes.layout.vf2_utils.score_layout() which is not documented in the published api docs and I wrote to be internal but under the deprecation policy wording would be viewed as public. My thinking by explicitly saying documented is to tighten the potential surface and make it clear what's expected to be treated as public.

I'll reword this a bit to be more clear, because I think there are some cases where private methods are part of a stable interface. The best example I can think of are things like interface definitions that have abstract private methods that need to be defined.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, there are a couple of cases where _* names are public:

  • Advanced-usage low-level methods we commit to stability in, for downstream code to use if they guarantee to uphold the class/safety invariants themselves. The archetypal example of this is QuantumCircuit._append, which is meant to be documented, but I think Sphinx might be overlooking it at the moment.
  • Abstract methods in base classes that are explicitly meant for subclasses to override. These are often prefixed with an underscore to show that users aren't meant to call them directly, but the interface classes call them. Things like SingletonInstruction._singleton_lookup_key fall into this category.

(with a ``_`` prefix). The supported Python versions, minimum supported Rust
version (for building Qiskit from source), and any depedency Python packages
mtreinish marked this conversation as resolved.
Show resolved Hide resolved
(including the minimum supported versions of dependencies) used by Qiskit are
not part of the backwards compatibility guarantees and may change during any
release. Only minor or major version releases will raise minimum requirements
wshanks marked this conversation as resolved.
Show resolved Hide resolved
for using or building Qiskit (including adding new dependencies), but patch
fixes might include support for new versions of Python or other dependencies.

<TODO Add calendar diagrams>

With the release of a new major version, the previous major version is supported
for at least 6 months; only bug and security fixes will be accepted during this
time and only patch releases will be published for this major version. A final
patch version will be published when support is dropped and that release will
also document the end of support for that major version series. A longer
Copy link
Contributor

Choose a reason for hiding this comment

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

What does it mean that it documents the end of the release? Its release notes will say that there is no more support? If there are no bug fixes at that time, will it just be identical to the previous patch release other than the release notes?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just the release notes. I was thinking we'd just put in the prelude something like: "0.46.5 is the final release of the 0.x release series of Qiskit. There will be no future releases of 0.x and you should migrate to using a 1.x release."

support window is needed for the previous major version as this gives downstream
consumers of Qiskit a chance to migrate not only their code but also their
users. It's typically not acceptable for a downstream library maintainer that
depends on Qiskit to immediately bump their minimum Qiskit version to a new
major version release immediately because their user base also needs a chance
to migrate to handle the API changes. By having an extended support window
for the previous major version that gives downstream maintainers to fix
compatibility the next major version but potentially keeping support for > 1
release series at a time and giving their users a migration path.
mtreinish marked this conversation as resolved.
Show resolved Hide resolved

Upgrade Strategy
================

Whenever a new major version is released the recommended upgrade path
is to first upgrade to use the most recent minor version on the previous major
version. Immediately preceding a new major version a final minor version will
be published. This final minor version release ``0.N+1.0`` is equivalent to
``0.N.0`` but with warnings and deprecations for any API changes that are
mtreinish marked this conversation as resolved.
Show resolved Hide resolved
made on the new major version series.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the "deprecation policy" section below might need to be updated to account for this final minor version and the new use of major versions. It talks about removing features only after providing an alternate path for one minor, warnings for one minor, and then removal at least two minors after the warnings. I think the idea is that with this final minor Qiskit can issue warnings for things removed in the next major even though they are released almost simultaneously because the final minor has an extended support window? I am not sure about the requirement to provide an alternate path before removal under the new semantic versioning model. If removing a feature in Qiskit 2.0, does anything have to happen in Qiskit 1 besides a deprecation in the final minor version?

Copy link
Member Author

@mtreinish mtreinish Nov 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I was thinking that ideally we'd still be adding the warnings and the alternative features in minor version releases ahead of the the 1->2 migration. In those cases we'd still want to follow that guidance. But in general yeah you're right there isn't a hard requirement for that, especially at the 1->2 migration point. I'll update the deprecation section to take this into account.


For example, on the release of Qiskit 1.0.0 a 0.46.0 release was published
immediately proceeding the 1.0.0 release. The 0.46.0 release was equivalent
to the 0.45.0 release but with additional deprecation warnings that documents
the API changes that are being made as part of the 1.0.0 release. This pattern
mtreinish marked this conversation as resolved.
Show resolved Hide resolved
will be used for any future major version releases.

As a user of Qiskit it's recommended that you first upgrade to this final minor
version first, so you can see any deprecation warnings and adjust your Qiskit
usage ahead of time before trying a potentially breaking release. The previous
major version will be supported for at least 6 months to give sufficient time
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think anything needs to change, but I am just noting something that I hadn't thought about before -- the version with overlapping support will be the one with all the deprecation warnings. That is probably fine as a push to upgrade to the new version. It could be noisy depending on the number of deprecations.

to upgrade. A typical pattern to deal with this is to pin the max version to
avoid using the next major release series until you're sure of compatibility.
For example, specifying in a requirements file ``qiskit<2`` will ensure that
mtreinish marked this conversation as resolved.
Show resolved Hide resolved
you're using a version of Qiskit that won't have breaking API changes.
wshanks marked this conversation as resolved.
Show resolved Hide resolved

Pre-releases
============

For each minor and major version release Qiskit will publish pre-releases that
are compatible with `PEP440 <https://peps.python.org/pep-0440/>`__. Typically
these are just release candidates of the form ``1.2.0rc1``. The ``rc`` will have
mtreinish marked this conversation as resolved.
Show resolved Hide resolved
a finalized API surface and are used to test a prospective release.

If another PEP440 pre-release suffix (such as ``a``, ``b``, or ``pre``) are
published these do not have the same guarantees as an ``rc`` release, and are
just preview releases. The API likely will change between these pre-releases
and the final release with that version number. For example, ``1.0.0pre1`` has
a different final API from ``1.0.0``.

Post-releases
=============

If there are issues with the packaging of a given release a post-release may be
issued to correct this. These will follow the form ``1.2.1.1`` where the fourth
integer is used to indicate it is the 1st post release of the ``1.2.1`` release.
For example, the qiskit-terra (the legacy package name for Qiskit) 0.25.2
release had some issue with the sdist package publishing and a post-release
0.25.2.1 was published that corrected this issue. The code was identical, and
0.25.2.1 just fixed the packaging issue for the release.

##################
Deprecation Policy
##################
Expand All @@ -21,17 +114,20 @@ underscore (``_``).

The guiding principles are:

- we must not remove or change code without active warnings for least three
months or two complete version cycles;
- we must not remove or change code without active warnings on a supported
release series for at least three months and removals can only occur on
Copy link
Contributor

Choose a reason for hiding this comment

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

The three month window is a little unclear to me. Regarding removals, couldn't it just be said that removals should only be done in the next major release, without regard to a window? And should changes be allowed with a three month warning? Or should they just be kicked to the next major release as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll just remove this. I left it in because in the semantic versioning model everything will be deprecated for at least 6 months because of the extended support. I was thinking we still wanted to have a minimum deprecation window duration, but in the model there isn't really path for it to be < 6 months so this isn't needed anymore.

major version releases;

- there must always be a way to achieve valid goals that does not issue any
warnings;

- never assume that a function that isn't explicitly internal isn't in use;

- all deprecations, changes and removals are considered API changes, and can
only occur in minor releases not patch releases, per the
:ref:`stable branch policy <stable_branch_policy>`.
- all deprecations can only occur in minor version releases not patch version
releases, per the :ref:`stable branch policy <stable_branch_policy>`.

- API changes and removals are considered breaking changes, and can only
occur in major version releases.

.. _removing-features:

Expand Down
17 changes: 15 additions & 2 deletions docs/maintainers_guide.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,27 @@ change:
a change already merged onto ``main``, unless the change simply does
not make sense on ``main``.

At any given time Qiskit has up to two supported stable branches, one for the
previous minor version release series and the other for the final minor
release series on the previous major release. For example, if the ``main``
branch is developing the 1.2.0 release, then the ``stable/1.1`` branch and
potentially the ``stable/0.46`` branches are supported.

Backporting
-----------

When a PR tagged with ``stable backport potential`` is merged, or when a
merged PR is given that tag, the `Mergify bot <https://mergify.com>`__ will
open a PR to the current stable branch. You can review and merge this PR
like normal.
open a PR to the most recent supported stable branch. You can review and merge
this PR like normal. If you're backporting from main to a different stable
branch you can leave a comment of the form ``@Mergifyio backport branch`` on a
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be worth adding a previous stable backport potential label to .mergify.yml?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, yeah this is a good idea we can totally do this. I'll update the docs to include this.

Copy link
Member

Choose a reason for hiding this comment

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

If we're going this route, I wouldn't mind doing it as explicit labels (backport to 0.45, backport to 1.1, etc) - it's more work to manage Mergify since we can't easily make it dynamic on the label, but I already sometimes forget what "stable backport potential" means when we're in an rc period.

merged PR to manually have the Mergify bot backport the PR to the specified
branch.

If the version of the code has diverged or no longer exists on the stable
branch it is acceptable to directly propose a change to the stable branch. But
it is expected to document the reasons why in the commit message, as the
typical procedure is to merge on ``main`` first.


Documentation Structure
Expand Down