-
Notifications
You must be signed in to change notification settings - Fork 0
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
Misc cleanups and improvements #32
Conversation
This will allow cloning of this repository (once it's public) without having to authenticate to GitHub.
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.
Hi @ibbem,
thank you for this pull request. Your changes are highly appreciated. :) I left some comments and questions below.
Also, I am unsure about the |
Yes we should do. I am currently still thinking about a proper name. :( |
That would be great! We probably need such an example / setup instructions for a potential artifact submission later on. |
All other `VariabilityLanguage`s use a single `L` prefix and it was already renamed in the reexport of `Lang.All`.
The interface of `cappedFin` using an implicit parameter which also doesn't represent the predecessor of the maximum value is very nice to use. Most uses in the code are already using `cappedFin`. `minFinFromLimit` also implements the same functionality but is currently unused and it seems like proofs about it are harder than for `cappedFin`.
fa495e3
to
2382b67
Compare
The option Index : Set₁
Index = Set → Set
data Example : Index → Set where
value : ∀ i → Example i Here, I just noticed that the option which conflicts with We can either avoid |
Ok interesting. Maybe that is a reason to avoid The idea to add the options to the library file sounds good to me. Someone should build an explicit feature model for Agda. :D |
In some places the new option `--large-indices` is required because an inconsistency with `--forced-argument-recursion` (which is enabled by default) was discovered.
Untracked files shouldn't influence the build. Hence, it's a good idea to ensure that they can't.
This also installs all Agda and Agda interface files instead of, like before, only keeping the executable.
This is currently an experimental Nix feature. However, it's already widely used so it makes sense to offer such a standardized interface.
The only exception is `--allow-unsolved-metas` because it's useful to know where unsolved problems might hide.
2382b67
to
0acd639
Compare
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.
You can merge when you think the PR is ready. :) Some items in our discussion might be better suited for future PRs.
These are mostly just some simple cleanups left on my TODO list.
The exception is 47321e3 which updates Agda and the standard library. I think that's a good idea because, first of all, this will hopefully get rid of some bugs we encountered. We might also get some other improvements (e.g. a performance improvement on NixOS). And lastly, this is probably better for the first impression: When people look at a library and it already uses some outdated dependencies, it leaves the impression that the library is already unmaintained.
The most incompatibilities I encountered where:
--large-indices
because we make use of it in some places (it was implicitly enabled by default before)._≡˘⟨_⟩_
to the new syntax_≡⟨_⟨_
(the standard library changed this for a nice alignment and symmetry)Related to 8fa5f9f: Should we rename EPVL to something else? I think this name stands for the old paper title "Expressive Power of Variability Languages" right?
Should the README include an example for how to use this library as a dependency in Nix (ba2a8f0)?