Skip to content

Fix breaking introduction of variable in Semiring.Primality #2774

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 1 commit into from
Jul 18, 2025

Conversation

MatthewDaggitt
Copy link
Contributor

Fixes accidental breaking change in #2772.

@MatthewDaggitt MatthewDaggitt added this to the v2.3 milestone Jul 18, 2025
@MatthewDaggitt MatthewDaggitt linked an issue Jul 18, 2025 that may be closed by this pull request
Copy link
Contributor

@jamesmckinna jamesmckinna left a comment

Choose a reason for hiding this comment

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

For the sake of v2.3, I'm happy to revert a prior change.

But the general principle of avoiding implicits in favour of variables is still a good one, and will, from time to time, introduce breaking changes because of re-ordering of telescopic bindings.

User code which relies on explicit instantiation of implicit arguments is inherently brittle, unless this points to the need to make the argument explicit, which should be flagged as a separate issue?

I hope that for v3.0 we can make all of this more systematic?

@MatthewDaggitt
Copy link
Contributor Author

I agree with all your points, particularly about how using implicits widely is very fragile! But I do think that we should be careful in introducing variables in minor version bumps and try to prioritise making breaking ones in major versions bumps nonetheless.

@MatthewDaggitt MatthewDaggitt added this pull request to the merge queue Jul 18, 2025
Merged via the queue into master with commit 1029995 Jul 18, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maybe avoid introducing almost undefined arguments?
2 participants