Skip to content

[Merged by Bors] - fix: shake bug, improve noshake.json generation#9772

Closed
digama0 wants to merge 11 commits intomasterfrom
shake_bug
Closed

[Merged by Bors] - fix: shake bug, improve noshake.json generation#9772
digama0 wants to merge 11 commits intomasterfrom
shake_bug

Conversation

@digama0
Copy link
Copy Markdown
Member

@digama0 digama0 commented Jan 16, 2024

This is the noshake.json file corresponding to the edits in leanprover-community/batteries#533. There was a small bug in shake making it much more conservative than it needed to be, so there is a big mathlib PR #9830, on which this PR depends because CI will not pass without it.


Open in Gitpod

@grunweg
Copy link
Copy Markdown
Contributor

grunweg commented Jan 16, 2024

I had idly wondered (in light of e.g. #9559 or other drive-by import minimisation) if there was such a bug. This is great news, I look forward to the actual adjusting PR!

@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Jan 16, 2024

bors d+

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Jan 16, 2024

✌️ digama0 can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@github-actions github-actions bot added the delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). label Jan 16, 2024
@ghost ghost added blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Jan 18, 2024
mathlib-bors bot pushed a commit that referenced this pull request Jan 19, 2024
This uses the improved `shake` script from #9772 to reduce imports across mathlib. The corresponding `noshake.json` file has been added to #9772.



Co-authored-by: Mario Carneiro <di.gama@gmail.com>
@ghost ghost removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) labels Jan 19, 2024
@ghost
Copy link
Copy Markdown

ghost commented Jan 19, 2024

This PR/issue depends on:

riccardobrasca pushed a commit that referenced this pull request Jan 19, 2024
This uses the improved `shake` script from #9772 to reduce imports across mathlib. The corresponding `noshake.json` file has been added to #9772.



Co-authored-by: Mario Carneiro <di.gama@gmail.com>
@ghost ghost added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 20, 2024
@sgouezel
Copy link
Copy Markdown
Contributor

What about merging master, fixing the conflict, and then sending this one to bors (it has already been approved by Scott)? I expect the longer you wait, the more painful it will get.

@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Jan 23, 2024

I've merged master and fixed the conflict, re-minimising imports there by hand.

bors merge

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Jan 23, 2024

👎 Rejected by label

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Jan 23, 2024
@kim-em kim-em removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jan 23, 2024
@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Jan 23, 2024

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Jan 23, 2024
- [ ] depends on: #9830

This is the `noshake.json` file corresponding to the edits in leanprover-community/batteries#533. There was a small bug in `shake` making it much more conservative than it needed to be, so there is a big mathlib PR #9830, on which this PR depends because CI will not pass without it.



Co-authored-by: Mario Carneiro <di.gama@gmail.com>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Jan 23, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title fix: shake bug, improve noshake.json generation [Merged by Bors] - fix: shake bug, improve noshake.json generation Jan 23, 2024
@mathlib-bors mathlib-bors bot closed this Jan 23, 2024
@mathlib-bors mathlib-bors bot deleted the shake_bug branch January 23, 2024 06:04
mathlib-bors bot pushed a commit that referenced this pull request Jan 23, 2024
Some of these are already transitively imported, others aren't used at all (but not handled by noshake in #9772).

Mostly I wanted to avoid needing all of algebra imported (but unused!) in `FilteredColimitCommutesFiniteLimit`; there are now some `assert_not_exists` to preserve this.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
mathlib-bors bot pushed a commit that referenced this pull request Feb 2, 2024
- [X] depends on: #9830
- [X] depends on: #9772
- [X] depends on: #9996

This checks files for unused imports. The output here is piped through `gh-problem-matcher-wrap` so that it will show up as annotations.



Co-authored-by: Mario Carneiro <di.gama@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants