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

Adapt w.r.t Coq/Coq#18164 #130

Merged
merged 1 commit into from Nov 11, 2023
Merged

Conversation

Villetaneuse
Copy link

@Villetaneuse Villetaneuse commented Oct 17, 2023

This prepares the removal of some deprecated files in 8.16.
See coq/coq#18164
Sorry to bother you @ckeller @vblot

This prepares the removal of some files which are deprecated in 8.16.
@proux01
Copy link

proux01 commented Oct 18, 2023

@Villetaneuse looking at the long list of commits, I guess this should target the coq-master branch rather than coq-8.13 (you can change that by clicking the "Edit" button on the right of the title on top of the page)

@Villetaneuse
Copy link
Author

Villetaneuse commented Oct 18, 2023

@Villetaneuse looking at the long list of commits, I guess this should target the coq-master branch rather than coq-8.13 (you can change that by clicking the "Edit" button on the right of the title on top of the page)

Thank you. @SkySkimmer maybe something needs to be updated. I ended up here with make ci-smtcoq. And the "default" branch is 8.13 in smtcoq.

@Villetaneuse Villetaneuse changed the base branch from coq-8.13 to coq-master October 18, 2023 15:48
@proux01
Copy link

proux01 commented Oct 18, 2023

@Villetaneuse looking at the long list of commits, I guess this should target the coq-master branch rather than coq-8.13 (you can change that by clicking the "Edit" button on the right of the title on top of the page)

Thank you. @SkySkimmer maybe something needs to be updated. I ended up here with make ci-smtcoq. And the "default" branch is 8.13 in smtcoq.

Nothing to update (on Coq side at least), there is no reason for the default branch of a repo to be the one used in Coq's CI.

@Villetaneuse
Copy link
Author

Just a gentle reminder @ckeller @vblot; could you please consider this PR for smtcoq?

@ckeller
Copy link
Contributor

ckeller commented Nov 11, 2023

@Villetaneuse Sorry, I was expecting the usual "please merge now" message (I thought it was the standard?).
I do not know why the workflow fails as it goes through on my pc, merging.

@ckeller ckeller merged commit 70ec292 into smtcoq:coq-master Nov 11, 2023
0 of 2 checks passed
@Villetaneuse
Copy link
Author

@Villetaneuse Sorry, I was expecting the usual "please merge now" message (I thought it was the standard?). I do not know why the workflow fails as it goes through on my pc, merging.

Thank you. Sorry if I don't use standard messages, I'm still a bit new to this.

@proux01
Copy link

proux01 commented Nov 14, 2023

I was expecting the usual "please merge now" message

Just to explain the misunderstanding: there are indeed two kinds of overlays for coq:

  • ones that have to be merged in sync with the upstream PR in coq, those are usually the one involving ML code, hence the most commons for plugins like smtcoq
  • ones that are backward compatible and should be merged at any time before the upstream PR in coq, those are usually the ones involving .v files, like the current PR

A good practice to avoid confusion is to state explicitly in the top message of the PR which of the two kinds it is and when merge is expected.

@ckeller
Copy link
Contributor

ckeller commented Nov 14, 2023

Many thanks @proux01 for claryfying. I will also make sure I understand correctly when a new PR is proposed.
Thank you all for the work!

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.

None yet

3 participants