-
Notifications
You must be signed in to change notification settings - Fork 236
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
Add properties of non-divisibility to Algebra.Properties.Magma.Divisibility
#2469
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes here are good.
Yes, I'd say that both points you raise are bugs that should be fixed (but in separate PRs). Make new issues? |
Well, the second is somehow already covered by the quoted issue (I'm not clear about the history of these things, nor how these examples here somehow slipped through the net thrown by #2341 ?) I've updated the preamble to link to a new issue for the first one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to add a CHANGELOG entry for the deprecations but apart from that looks good!
Fixes #2306
NB.
Relation.Binary.Definitions._Respects₂_
seems to exchange 'left' and 'right' in its left/right projections? UPDATED:Relation.Binary.Definitions._Respects₂_
seems to exchange 'left' and 'right' in its left/right projections? #2471∣∣-resp*-≈
lemmas concerning_∣∣_
all explicitly mention the equality_≈_
, while none of the others (new, and existing) do? cf. Consistently add the equality afterresp
in proof names. #2341 Is this the occasion to remedy that state of affairs (in whichever direction we think: @MatthewDaggitt 's preference on that issue is to retain the symbol, so some additional renaming/deprecation would be necessary here...) UPDATED now FIXED, but it does feel like a lot of extra 'ink'?