Skip to content

Ensure equal versions have equal hashes #283

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

Merged
merged 3 commits into from
Oct 20, 2020
Merged

Ensure equal versions have equal hashes #283

merged 3 commits into from
Oct 20, 2020

Conversation

sbrudenell
Copy link
Contributor

Per the python datamodel, a == b implies hash(a) == hash(b). The current implementation of VersionInfo.__hash__ includes the build, which isn't used for equality.

@tomschr
Copy link
Member

tomschr commented Oct 15, 2020

@sbrudenell Thanks for this contribution! Much appreciated! 👍

I like it. 👍 My only concern is that this change breaks compatibility with previous releases, right? Maybe this change should go into semver 3?

Futhermore, we should document this behavior somewhere in docs/usage.rst. Maybe in Comparing Versions or in a separate section.
It doesn't need much. I would describe a general example and then give the specific one with a build part. That should include a text that the build part isn't taken into account.

Does that make sense to you?

@tomschr tomschr added the Enhancement Not a bug, but increases or improves in value, quality, desirability, or attractiveness label Oct 15, 2020
@sbrudenell
Copy link
Contributor Author

IMO it's a stretch to say this change breaks compatibility. Technically true, but any code relying on the current behavior is relying on the python data model being broken, which is pathological enough that I wouldn't bother supporting it. Up to you though.

Is there a separate branch for changes going into 3.x?

I'll update the docs.

@tomschr
Copy link
Member

tomschr commented Oct 15, 2020

IMO it's a stretch to say this change breaks compatibility. [...]

Yes, maybe my question was not entirely correct. It was more of a "thinking out loud" approach and to get your opinion. Probably I'm too concerned about that, maybe it's not a big issue. 😉 But it's something that I consider first before accepting any PR. So let me think about that.

Is there a separate branch for changes going into 3.x?

At the moment this is not the case, unfortunately. I hope we can leave semver2 behind soon and start working on semver 3. For you, nothing changes at the moment.

I'll update the docs.

That would be great! 👍

Copy link
Member

@tomschr tomschr left a comment

Choose a reason for hiding this comment

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

Thanks @sbrudenell for your additions. Much appreciated.

I have some minor comments, maybe we can work together to solve them. :-)

@tomschr
Copy link
Member

tomschr commented Oct 20, 2020

I'm currently thinking if this change could be safely integrated into the current semver2 code stream or better added to semver3 only.

@tlaferriere @python-semver/reviewers Thoughts?

@tlaferriere
Copy link
Contributor

This does feel like a bugfix that could be integrated in the 2.12.x line. All we have to do is implement it 😉. So much for freezing 2.10.x 😂.
I don't have much spare time atm but I see you're on a roll @tomschr so if you feel like getting that done, it's up to you to decide whether you want to release it as a bugfix or in in the next major change.

@tomschr
Copy link
Member

tomschr commented Oct 20, 2020

I've tried to read the semver spec to find a dedicated section regarding to compare two versions. The closed I could find was §10:

10. [...] Build metadata MUST be ignored when determining version precedence.
Thus two versions that differ only in the build metadata, have the same precedence.

Although the spec talks about version precedence, I think, this can be interpreted as our use case. That makes this a bugfix that should be integrated into semver2 and 3.

Thanks @sbrudenell and @tlaferriere for your contributions and help! Much appreciated! 👍

@tomschr tomschr merged commit db55f27 into python-semver:master Oct 20, 2020
tomschr added a commit to tomschr/python-semver that referenced this pull request Oct 20, 2020
* python-semver#283: Ensure equal versions have equal hashes
@tomschr tomschr mentioned this pull request Oct 20, 2020
tomschr added a commit to tomschr/python-semver that referenced this pull request Oct 20, 2020
* python-semver#283: Ensure equal versions have equal hashes
tomschr added a commit to tomschr/python-semver that referenced this pull request Oct 20, 2020
* python-semver#283: Ensure equal versions have equal hashes
tomschr added a commit to tomschr/python-semver that referenced this pull request Oct 20, 2020
* python-semver#283: Ensure equal versions have equal hashes
tomschr added a commit to tomschr/python-semver that referenced this pull request Oct 20, 2020
* python-semver#283: Ensure equal versions have equal hashes
* Update list of contributors
tomschr added a commit that referenced this pull request Oct 20, 2020
* #283: Ensure equal versions have equal hashes
* Update list of contributors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Not a bug, but increases or improves in value, quality, desirability, or attractiveness
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants