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

bump package version to 0.21.0 #138

Merged
merged 2 commits into from
Nov 1, 2023

Conversation

wallies
Copy link
Collaborator

@wallies wallies commented Oct 9, 2023

No description provided.

@wallies wallies requested review from cjrh and Sidhant29 October 9, 2023 05:21
@wallies
Copy link
Collaborator Author

wallies commented Oct 9, 2023

@adamreichold @GodTamIt @cjrh @Sidhant29 Be good to get your perspective. Do we need to look at this naming scheme e.g. 0.21.0 tantivy version with an extra .x for tantivy-py updates or do we not need to solve for this

@adamreichold
Copy link
Collaborator

While having the versions of the tantivy crate and the tantiv-py package agree is nice, I consider a bit of coincidence and hence would not go against the established semver practice of using major.minor.patch versions just to accommodate it.

@cjrh
Copy link
Collaborator

cjrh commented Oct 9, 2023

@adamreichold I disagree. The long history of the APSW package, and my user experience of working with that package over many years, has made it quite clear that for wrappers, having a simple correspondence between the inner package (SQLite) and the wrapper package (APSW) is valuable.

They describe it here: https://rogerbinns.github.io/apsw/about.html#apsw-and-sqlite-versions

It may not be strict semver but semver doesn't have a good solution for wrapper packages anyway.

@cjrh
Copy link
Collaborator

cjrh commented Oct 9, 2023

Here, recently, the arrow-datafusion-python wrapper discussed the same problem: apache/datafusion-python#123

Here, several years ago, a comment at semver saying that semver is not intended to describe package dependences. However, they offer the idea of, e.g. M1.m1.p1+tantivy.M2.m2.p2 as a way of representing both (M1 for tantivy-py major version, M2 for tantivy major version). It looks terrible though :)

apache/datafusion-python#123

@cjrh
Copy link
Collaborator

cjrh commented Oct 9, 2023

APSW used to use M.m.p-r1, in this issue they described why to move away from that to the system they now use.

@GodTamIt
Copy link
Contributor

This might be a bit of a bikeshed but I would say that having tantivy and tantivy-py version numbers match would be my preference. Chief among the reasons is that it makes tracking bugs and version history significantly easier.

However, I don't have strong opinions on how we'd differentiate between builds of tantivy-py with the same underlying tantivy version.

@cjrh
Copy link
Collaborator

cjrh commented Oct 31, 2023

Summarizing the thread so far:

  • semver-only: If we stick to strict semver major.minor.patch, then we will disregard the version of the tantivy crate being wrapped. All the rules of semver then apply as-is, in exchange for losing an easy-to-see understanding of which version of tantivy is included within which version of tantivy-py.
  • no-semver: If we give up semver compliance, using something like tantivy-major.tantivy-minor.tantivy-patch.tantivy-py-patch, then we gain the easy-to-see tantivy version number, but we lose semver communication. Users of tantivy-py will be unable to infer the semver-impact of a change in the value of the tantivy-py-patch number. We could attempt to reuse the semver change levels of the inner tantivy, e.g., if tantivy-minor changes, then we would also "be allowed" to make changes compatible with semver minor changes; but this is very messy and difficult to articulate. From a consumer perspective, if you're not strictly semver all bets are kinda off.
  • super-semver: Finally we have the compromise option: M1.m1.p1+tantivy.M2.m2.p2. This is the most complete but also the most verbose. This is however compatible with strict semver and has been suggested by the semver project as a potential solution for wrapper libraries. The first triple applies to tantivy-py, and is strict semver, while the +tantivy.M2.m2.p2 are the tantivy library numbers. This would allow us to, for example, update the underlying tantivy crate to a new version without changing the tantivy-py version component.

I can live with semver-only, and it would be more palatable if we can make the tantivy-py version number look different from the tantivy version number. I don't have good ideas here for how to do that. This option has the benefit of being overall simpler from the point of view of just managing the version number. We lose only the human inconvenience of being able to tell at a glance which version of tantivy is included.

I have a lot of experience with the no-semver model, since this is what the APSW Python library has used for many years. It has helped that APSW has been very conservative and stable in their development and release process. It may be that this is why the version number not being semver has caused few issues. Thinking it over, if we had had this method from the start, it would not have been super useful, because the availability of features in tantivy-py has lagged behind the features in the included tantivy crate anyway. So the value in showing the version of the inner crate would be questionable.

I can also live with the super-semver version. I don't have experience with it, so others will have to chime in.

My opinion is that I am moving towards just doing semver as @adamreichold suggested. We can help with the discoverability of the inner tantivy version by making a top-level module function that reports it. This is easily used in a REPL, and can easily be collected during the documentation build.

@wallies wallies changed the title bump package version to 0.21.0.0 bump package version to 0.21.0 Oct 31, 2023
@cjrh
Copy link
Collaborator

cjrh commented Nov 1, 2023

Ok @wallies let's move forward with semver.

@wallies wallies merged commit cdb2aec into quickwit-oss:master Nov 1, 2023
@wallies wallies deleted the bump-version-0.21.0 branch November 1, 2023 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants