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

Update OpenAPI schemas for v0.13.2 of python-tools #350

Merged
merged 5 commits into from
Mar 3, 2021

Conversation

ml-evs
Copy link
Member

@ml-evs ml-evs commented Feb 17, 2021

Just a minor change that adds the species->mass field from #344.

@ml-evs ml-evs changed the title Update OpenAPI schemas for v0.12.10 of python-tools Update OpenAPI schemas for v0.13.0 of python-tools Feb 19, 2021
@ml-evs ml-evs marked this pull request as ready for review February 20, 2021 22:15
merkys
merkys previously approved these changes Feb 23, 2021
Copy link
Member

@merkys merkys left a comment

Choose a reason for hiding this comment

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

Seems OK to me!

@ml-evs ml-evs changed the title Update OpenAPI schemas for v0.13.0 of python-tools Update OpenAPI schemas for v0.13.1 of python-tools Feb 23, 2021
@ml-evs ml-evs added this to the 1.0.1 release milestone Feb 26, 2021
@ml-evs ml-evs added PR/minor-fixes-only PR/ready-for-review Add this flag if you are the author of the PR and you want it to be reviewed. Remove it when editing blocking-release This is a PR or issue that presently blocks the release of next version of the spec. labels Feb 26, 2021
@ml-evs ml-evs changed the title Update OpenAPI schemas for v0.13.1 of python-tools Update OpenAPI schemas for v0.13.2 of python-tools Mar 1, 2021
merkys
merkys previously approved these changes Mar 2, 2021
Copy link
Contributor

@rartino rartino left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for updating the schemas and accommodating my ranting about version numbers :-)

@ml-evs ml-evs requested a review from merkys March 2, 2021 13:31
Copy link
Member

@CasperWA CasperWA left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I am wondering whether we should avoid using "post-release" versions and instead follow implemented Semantic Versioning concepts such as "pre-release" versions instead?

A way that this could be realized would be to change the develop branch to use a pre-release version according to the SemVer specification, with a scheme that best fits our needs. I would suggest to add a "second" semantic version, upping the patch version of the proper version. Example:

We release a new version, say 1.1.0, and update the develop branch to have the version 1.1.1-0.1.0. Essentially meaning version 0.1.0 of the 1.1.1 release.
If we later find that we need to up the public version's minor version to 1.2.0, there is no reason why we cannot finalize the develop branch before a new release, updating the version accordingly to 1.2.0, then again updating develop to 1.2.1-0.1.0 again.
This will essentially mean that the develop branch is always developing the next patch version, with the reservation that we can, at any time, collect the develop commits to release a new minor or major public version.
Furthermore, we can up the pre-release version according to the merged PRs. This means everyone will have to evaluate the impact of their PR, which might not be bad, but of course risks that the barrier is increased further, minimizing contributions - but we could assist in this assessment and always add a commit ourselves before merging that properly updates the versioning (following the latest example above, one would update the develop version for a minor change accordingly: 1.2.1-0.1.0 becomes 1.2.1-0.2.0, and major/patch changes will work in a similar way).

What do you think?

@ml-evs ml-evs merged commit 3acc092 into develop Mar 3, 2021
@CasperWA
Copy link
Member

CasperWA commented Mar 3, 2021

Note, after reading a bit in the old issue #255 I still think a pre-release version is the way to go, even though we don't know the future and eventually released public version number.
The point of using the pre-release versioning, i.e., adding - tells that the specific version of the specification is not public and should be used in public, but only in development. It doesn't disallow us the freedom to publicly release a different version, i.e., one that doesn't just increase the patch version by one, but the minor or even major version. The trick of upping the "public" patch version number is simply to be sorted correctly according to the semver precedence rules, but the fact that a "main" version is never released with this patch version increase is irrelevant, as it's not a public version.

@CasperWA CasperWA deleted the python-tools-0.12.10-schema branch March 3, 2021 15:28
@rartino
Copy link
Contributor

rartino commented Mar 16, 2021

@CasperWA So, more concretly; lets say we have just released v1.2.3 out of the develop branch. What should now the version number be set to? v1.2.4-develop? If there are three PRs in the queue at that point that want to merge new features, should those PRs be updated to alter the version number in any way? (Which one? All of them? To what?)

What is the advantage of this scheme over v1.2.3~develop?

Since the discussion in #255 I've also come to realize that it is possible (in non-pathological cases) for situations where two PRs individually do not affect the semantic version. However, when both are merged, in either order, they may require a feature or major version bump. This makes it very tricky to tie version bumps to individual PRs - essentially the question of a version bump needs to finally be decided at merge time.

@CasperWA
Copy link
Member

@CasperWA So, more concretly; lets say we have just released v1.2.3 out of the develop branch. What should now the version number be set to? v1.2.4-develop? If there are three PRs in the queue at that point that want to merge new features, should those PRs be updated to alter the version number in any way? (Which one? All of them? To what?)

I think there is a misunderstanding of every PR having to update the version number here. That was not my intended workflow.
Rather, the master branch is the only branch that holds public versions (as tags to commit SHAs).
The develop branch holds, as an addition to what the master branch holds, a collection of commits that together constitutes a version to-be. The in-development version number for this is 1 patch version number higher than the latest released public version including a SemVer pre-release version tag (-develop). When it's time for a new release the new actual version number will be evaluated and will by SemVer's precedence rules always take precedence of the "develop" version as well as the previously latest public version up to this point.

So say we release v1.2.3. Then the develop branch will become 1.2.4-develop.
Then we wish to release v1.3.0 based on the commits currently in develop (but not in master), so we push a final commit to develop, which updates the version to 1.3.0, release it publicly, move it to master and update the develop branch with a new commit that uses version 1.3.1-develop.

While this will "invalidate" any versioning done in the develop branch that does not overlap with the master branch, it allows for proper API reference with SemVer syntax of which version one is pointing to.

What is the advantage of this scheme over v1.2.3~develop?

That any version reference will always follow proper SemVer syntax and will follow any precedence rules properly - even if one decides to publish a 1.2.3-develop version, for whatever reason.
The point of upping the patch version number by 1 is only meant to fit the version properly into the precedence rules, while acknowledging that the next public version can be at minimum at patch version release. There is no upper bound.

Since the discussion in #255 I've also come to realize that it is possible (in non-pathological cases) for situations where two PRs individually do not affect the semantic version. However, when both are merged, in either order, they may require a feature or major version bump. This makes it very tricky to tie version bumps to individual PRs - essentially the question of a version bump needs to finally be decided at merge time.

I think this again boils down to the fact that you wish a version bump with (and version evaluation of) every new PR. This would indeed be nice, but my suggested workflow does not rely on this. It relies on doing this evaluation only when actually releasing a new version, while keeping everything appliant with SemVer versioning precedence and syntax.

@rartino
Copy link
Contributor

rartino commented Mar 16, 2021

Ok, so to clarify: the scheme you propose is the exact same scheme as we have right now, except you propose to consistently replace 'v.1.2.3~develop' with 'v1.2.4-develop'. The argument for this somewhat minor change is:

any version reference will always follow proper SemVer syntax and will follow any precedence rules properly - even if one decides to publish a 1.2.3-develop version, for whatever reason.

But, on the other hand, this is formally an invalid use of SemVer version numbers. SemVer versions are versions of releases. The continuously changing state of the development branch is not a release, and it is arguably a very bad idea to let all those different code states share a single SemVer version number.

To "decide to publish a '1.2.3-develop' version" is more or less a disaster of an idea, since it looks like a proper SemVer release, but could represent a range of actual code states; and there can even be more than one functionally different releases with that same version number. If such a release is made on purpose, it is better that the person releasing it is forced to give it a non-development version number. If the release is made by mistake, the mistake is made more clear by having a SemVer-breaking version string.

That said, I'm genuinely interested if there are more clear advantages. Do you have any better example of when proper SemVer-ordering of development revisions would be useful?

Advantages of 'v1.2.3~develop':

  • Clear interpretation: this is release v1.2.3 plus developement.
    • IMO far more self-explanatory than a non-standard convention of "subtract one from patch version to get the base release".
  • Any (automated) attempt to parse the version string of the development branch as a SemVer release breaks, which is a good thing.
  • May prevent confusion about whether PRs should update the version number (see below).

I think there is a misunderstanding of every PR having to update the version number here.

Ok, sorry. But, it is an easy misunderstanding to make, because if one digs down into what is suggested in this situation, one finds many who push precisely for this: every PR updates the version string according to the changes made in the PR. (I suspect many who push heavily for this are primarily used to few-people projects where one do not encounter the full difficulties in upholding the scheme with many parallel contributions.)

ml-evs added a commit that referenced this pull request Apr 23, 2021
* Update OpenAPI schemas for v0.13.0 of python-tools

* Bumped OPTIMADE spec version (1.0.1) and optimade-python-tools version (0.13.1)

* Use version number 1.0.1~develop

* Added regexp for prefix from v0.13.2 of python-tools

* Use 1.0.0~develop as the version tag
ml-evs added a commit that referenced this pull request May 30, 2021
* Update OpenAPI schemas for v0.13.0 of python-tools

* Bumped OPTIMADE spec version (1.0.1) and optimade-python-tools version (0.13.1)

* Use version number 1.0.1~develop

* Added regexp for prefix from v0.13.2 of python-tools

* Use 1.0.0~develop as the version tag
rartino pushed a commit that referenced this pull request Jul 7, 2021
* Update OpenAPI schemas for v0.13.0 of python-tools

* Bumped OPTIMADE spec version (1.0.1) and optimade-python-tools version (0.13.1)

* Use version number 1.0.1~develop

* Added regexp for prefix from v0.13.2 of python-tools

* Use 1.0.0~develop as the version tag
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking-release This is a PR or issue that presently blocks the release of next version of the spec. PR/minor-fixes-only PR/ready-for-review Add this flag if you are the author of the PR and you want it to be reviewed. Remove it when editing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants