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

Move bugs that have been closed on Bugzilla #500

Merged
merged 1 commit into from Apr 12, 2018

Conversation

tchajed
Copy link
Contributor

@tchajed tchajed commented Mar 22, 2017

To find these bugs, I searched for all the open bugs that are resolved by visiting the following URL, run from the root of the Coq sources.

$ echo 'https://coq.inria.fr/bugs/buglist.cgi?bug_id_type=anyexact&bug_status=RESOLVED&list_id=331382&product=Coq&query_format=advanced&resolution=FIXED&bug_id='$(ls test-suite/bugs/opened/*.v | grep -o '\d\+' | tr '\n' ',')

@tchajed tchajed changed the base branch from trunk to v8.6 March 23, 2017 13:26
@maximedenes
Copy link
Member

There is something odd here. Usually when moving a test case from open to closed, you need to invert the logic of the test (typically remove Fail).

The test case 1501.v is incorrect. In fact, I'm guessing the bug should have been closed with INVALID.
2456.v still fails, for a different reason, so it may be a regression.
2814.v seems a legitimate move since it now fails whereas it was emitting an anomaly before.
3100.v is similar
3320.v is probably not really testing the bug
3794.v I don't understand why the bug is closed.

@maximedenes maximedenes added the needs: fixing The proposed code change is broken. label Mar 29, 2017
@ejgallego ejgallego added this to the 8.7 milestone May 30, 2017
@maximedenes maximedenes modified the milestones: 8.8, 8.7 Jun 2, 2017
@Zimmi48 Zimmi48 added the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label Jul 18, 2017
@Zimmi48 Zimmi48 changed the base branch from v8.6 to master November 9, 2017 17:48
@Zimmi48
Copy link
Member

Zimmi48 commented Nov 29, 2017

Tagging as help wanted because I'm guessing the lack of response from @tchajed means that he doesn't really want to spend more time on this.

@tchajed
Copy link
Contributor Author

tchajed commented Jan 25, 2018

Ok, I checked the new list of resolved bugs in opened/. Here are the details from manually checking each bug:

not addressed:

@tchajed tchajed added needs: review and removed help wanted needs: fixing The proposed code change is broken. needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. labels Jan 25, 2018
@maximedenes maximedenes modified the milestones: 8.8+beta1, 8.8.0 Mar 7, 2018
@Zimmi48 Zimmi48 added this to Request 8.8 inclusion in Coq 8.8 Mar 22, 2018
Copy link
Member

@mattam82 mattam82 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine to me, indeed the changes to 1501 are alright. For issue #4813 this is another instance where a bidirectional typechecking flag or something more general to control inference would be useful.

@Zimmi48
Copy link
Member

Zimmi48 commented Mar 27, 2018

There is no code owner for the test-suite (maybe for the same reason that there is no code owner for CHANGES). @maximedenes do you want to take care of this PR?

@SkySkimmer SkySkimmer added needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. needs: owner PR only on free files. and removed needs: review labels Apr 1, 2018
@Zimmi48 Zimmi48 removed the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label Apr 11, 2018
@Zimmi48
Copy link
Member

Zimmi48 commented Apr 11, 2018

@mattam82 Since you approved this and it has no owner, will you take care of merging it?
@tchajed Can you rebase? It should be straightforward.

@Zimmi48 Zimmi48 added the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label Apr 11, 2018
@tchajed
Copy link
Contributor Author

tchajed commented Apr 11, 2018

Rebased.

@maximedenes
Copy link
Member

I'll merge, since this has been approved.

@maximedenes maximedenes self-assigned this Apr 12, 2018
@maximedenes maximedenes added kind: cleanup Code removal, deprecation, refactorings, etc. and removed needs: owner PR only on free files. needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. labels Apr 12, 2018
@maximedenes maximedenes merged commit 26d9acf into coq:master Apr 12, 2018
Coq 8.8 automation moved this from Request 8.8 inclusion to Waiting to be backported Apr 12, 2018
Zimmi48 added a commit to Zimmi48/coq that referenced this pull request Apr 12, 2018
@Zimmi48 Zimmi48 moved this from Waiting to be backported to Waiting for CI in Coq 8.8 Apr 12, 2018
@Zimmi48 Zimmi48 moved this from Waiting for CI to Shipped in 8.8.0 in Coq 8.8 Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: cleanup Code removal, deprecation, refactorings, etc.
Projects
No open projects
Coq 8.8
  
Shipped in 8.8.0
Development

Successfully merging this pull request may close these issues.

None yet

6 participants