Skip to content

Adapt w.r.t. coq/coq#16004. #111

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 2 commits into from
Jun 30, 2022

Conversation

ppedrot
Copy link
Contributor

@ppedrot ppedrot commented May 10, 2022

Should be backwards compatible.

@ppedrot
Copy link
Contributor Author

ppedrot commented May 10, 2022

Not sure what is the policy for older versions. I'm a bit surprised that mathclasses still compiles with 8.6, is this really true?

@spitters
Copy link
Collaborator

I don't recall dropping support for 8.6 to 8.10, but it looks like the nix file only tests from 8.11, which would be fine with me.

@ppedrot ppedrot force-pushed the hint-locality-error branch from 72ae762 to 7a8f515 Compare June 27, 2022 14:28
@ppedrot
Copy link
Contributor Author

ppedrot commented Jun 27, 2022

I've pushed a patch to be more compatible with older versions, by deactivating the unknown attribute warning. Let's see how it fares, if it compiles up to 8.11 it should be good to go.

@ppedrot ppedrot force-pushed the hint-locality-error branch from 7a8f515 to 9d3fc8e Compare June 27, 2022 14:36
@spitters
Copy link
Collaborator

Thanks! It works from 8.11.
We should probably disable the other builds in CI.

@ppedrot
Copy link
Contributor Author

ppedrot commented Jun 28, 2022

OK, so I consider this ready to merge. Deactivating the broken CIs should be performed in its own PR, instead.

@Zimmi48
Copy link
Member

Zimmi48 commented Jun 28, 2022

We had dropped compatibility with Coq versions < 8.11 in 8.13.0, but it was reintroduced by a contributor in 8.15.0. It's fine to remove it now. Removing the tests with Coq 8.6-8.10 should amount to reverting #103 (except that there will be a conflict in meta.yml and the opam file).

@ppedrot
Copy link
Contributor Author

ppedrot commented Jun 28, 2022

AFAICT even in master these tests are broken. If you look at the output of the CI in this PR, there is already an issue with the Automatic Introduction flag which is unrelated to the patches from here.

@ppedrot
Copy link
Contributor Author

ppedrot commented Jun 30, 2022

@spitters ping

@spitters
Copy link
Collaborator

Thanks! I'll merge this, @Zimmi48 if you have time, could you send a PR to reverse #103 ?

@spitters spitters merged commit 63f2130 into rocq-community:master Jun 30, 2022
@ppedrot ppedrot deleted the hint-locality-error branch June 30, 2022 09:52
Zimmi48 added a commit to Zimmi48/math-classes that referenced this pull request Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants