Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Refactor
Data.List.Relation.Binary.Permutation.*
, part I #2333Refactor
Data.List.Relation.Binary.Permutation.*
, part I #2333Changes from 30 commits
7ab1da5
4ece6bb
38f22e4
9399636
c6f701f
b37ba6f
791baf9
ff7845a
a3d0b8e
2950a39
69ca8fd
5f0051a
5461356
8b44fd8
7918a8d
00a335a
91feb30
227043d
4df26ef
0bf8e38
60a1958
fd80b0b
1781ea3
7c2bc35
2e032a5
4ad9fef
83dc6c7
246bf5c
39bb784
b7dff80
ce4d72f
c8bf0a6
e5701d9
a40a26f
cbef1f8
9eb7000
a5e2849
26371ae
dd296a6
7f84607
92c14a8
901310f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
As always I am going to be annoying and complain that I don't like prefixed
names because users can either use the short names or import the module
qualified if they need to manually disambiguate.
The same goes for
split
being renamed to↭-split
(although in that casethe name
split
is already pretty poor IMO).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.
@gallais I don't find the comment annoying!
What I find annoying is the the abstraction failure when explicitly exporting constructors from an inductive definition which should remain (externally, to clients) abstract, IMHO (cf.
z<s
inData.Nat.Base
, and0<1+n
inData.Nat.Properties
, not that I/we are entirely consistent in enforcing this...)Since this is also part of a greater project of refactoring
Permutation
towards completing #1761 / #1354 I wanted to make sure that inProperties
, references to the constructors were abstracted on those lines before proceeding further.As regards
↭-split
, this is not a renaming ofsplit
, but a significant generalisation which permits a much improved proof ofdropMiddleElement
... so a cognate name change is needed to reflect the 'better' typing! As for giving it a better name, I'm open to suggestions!