Skip to content
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

[fleche] Reject opening multiple Coq documents for buggy Coq versions. #287

Merged
merged 1 commit into from
Feb 7, 2023

Conversation

ejgallego
Copy link
Owner

@ejgallego ejgallego commented Feb 6, 2023

Fixes #275 , fixes #281

We use a heuristic in the fixed up branches as follows: if the Coq version contains LSP, then we assume the fix has been backported.

That should work when doing opam pin.

@ejgallego
Copy link
Owner Author

I will merge this (and release 0.1.5) when the branches noted in the README are ready.

@ejgallego
Copy link
Owner Author

@Alizter what do you think of this?

@ejgallego ejgallego requested a review from Alizter February 6, 2023 19:29
@Alizter
Copy link
Collaborator

Alizter commented Feb 6, 2023

Logic seems sound, but I haven't tested it myself.

@ejgallego
Copy link
Owner Author

ejgallego commented Feb 6, 2023

Logic seems sound, but I haven't tested it myself.

Thanks for the feedback @Alizter , indeed logic seems to work, I have done some testing, and IMHO it is a much better experience that what we have today; I hope the error message is Ok for users.

I'm waiting to see what's the outcome of coq/coq#17200 just in case we would only need this for the 8.16 branch.

Fixes #275 , #281

We use a heuristic in the fixed up branches as follows: if the Coq
version contains LSP, then we assume the fix has been backported.

That should work when doing `opam pin`.
@ejgallego ejgallego force-pushed the fail_hard_on_some_docs branch from 3f36a86 to 6dd0231 Compare February 7, 2023 00:28
@ejgallego ejgallego merged commit 990d263 into main Feb 7, 2023
@ejgallego ejgallego deleted the fail_hard_on_some_docs branch February 7, 2023 13:52
@ejgallego
Copy link
Owner Author

Merging now, we can adjust the notes once we discuss about the Coq backport, before release (no point on holding this due to code conflicts)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Syntax error for "assert ... by ... " Reject checking multiple documents when Coq < 8.18
2 participants