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

Remove deprecated files in Coq.Arith #47

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

Villetaneuse
Copy link
Contributor

@Villetaneuse Villetaneuse commented Oct 16, 2023

Necessary for coq/coq#18164

@Villetaneuse
Copy link
Contributor Author

@JasonGross This should be ok, I think.

@JasonGross
Copy link
Contributor

@Villetaneuse I don't have permissions on this repository, I think we need @samuelgruetter @andres-erbsen here

@JasonGross
Copy link
Contributor

Also, btw, if you write it as coq/coq#18164 (without the :), I think GitHub will autolink the issue

@Villetaneuse
Copy link
Contributor Author

Villetaneuse commented Oct 17, 2023

Also, btw, if you write it as coq/coq#18164 (without the :), I think GitHub will autolink the issue

Sorry, still quite new to this. This is edited. And thank you for the correct pings!

@samuelgruetter
Copy link
Contributor

In Coq 8.15, this fails with

Error: The reference Even_Odd_dec was not found in the current environment.

But, as far as I understand, the bbv library is not really used any more, except maybe for rebuilding old projects, so before anyone spends time on making it compatible with several Coq versions, maybe we should ask if we should simply take bbv out of Coq's CI?

@Villetaneuse
Copy link
Contributor Author

Villetaneuse commented Oct 18, 2023

In Coq 8.15, this fails with

Error: The reference Even_Odd_dec was not found in the current environment.

But, as far as I understand, the bbv library is not really used any more, except maybe for rebuilding old projects, so before anyone spends time on making it compatible with several Coq versions, maybe we should ask if we should simply take bbv out of Coq's CI?

Thank you, I don't know who decides such things. I'm putting a link to your post on zulip here.

@JasonGross
Copy link
Contributor

It's used by fiat-crypto-legacy, which I'd like to keep compatible in Coq's CI, but for which multiple version compatibility is also not important, so maybe it's best to bump bbv's CI to test 8.16, 8.17, 8.18, master?

@samuelgruetter
Copy link
Contributor

so maybe it's best to bump bbv's CI to test 8.16, 8.17, 8.18, master?

I just tried that, but it seems 8.18 is not yet available in the Ubuntu builds. Should I just do 8.16, 8.17 & master for now @JasonGross ?

@JasonGross
Copy link
Contributor

Yes, that sounds good. I'll try to get 8.18 in Ubuntu packaging soon

@samuelgruetter
Copy link
Contributor

Ok so I just pushed CI for 8.16, 8.17 & Coq master to bbv's master.
@Villetaneuse could you please rebase this PR?

Necessary for Coq/Coq:#18164
@Villetaneuse
Copy link
Contributor Author

Ok so I just pushed CI for 8.16, 8.17 & Coq master to bbv's master. @Villetaneuse could you please rebase this PR?

Done.

@samuelgruetter samuelgruetter merged commit f4caa05 into mit-plv:master Oct 19, 2023
3 checks passed
@Villetaneuse
Copy link
Contributor Author

@samuelgruetter thank you very much!

JasonGross added a commit to mit-plv/fiat-crypto that referenced this pull request Oct 22, 2023
JasonGross added a commit to mit-plv/fiat-crypto that referenced this pull request Oct 22, 2023
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