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

[Merged by Bors] - chore(Tactic/TypeStar): reduce imports #11876

Closed
wants to merge 5 commits into from

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Apr 3, 2024

I noticed that this doesn't actually use Std or in fact any of its other imports.

The downstream fallout is pretty small because Mathlib.Tactic.Basic imports Std anyway, so this only impacts files not downstream of that.


Open in Gitpod

@eric-wieser eric-wieser added awaiting-review The author would like community review of the PR easy < 20s of review time. See the lifecycle page for guidelines. awaiting-CI labels Apr 3, 2024
Copy link
Collaborator

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

Does what it says, and there seems to be support for and no resistance against this particular change on zulip.

Thank you for making this change.

@adomani
Copy link
Collaborator

adomani commented Apr 4, 2024

Thanks!

I see that there are additions to shake. Should there also be some deletions, from the no longer imported Std?

@eric-wieser
Copy link
Member Author

I just ran shake --update; I assume this auto-removes ignore instructions that no longer do anything?

@adomani
Copy link
Collaborator

adomani commented Apr 5, 2024

This is a question for @digama0. I suspect that an unused import should have an entry in shake and removing an unused import should therefore imply a removal in shake.

Anyway, this is certainly not an issue with this PR!

maintainer merge

@digama0
Copy link
Member

digama0 commented Apr 5, 2024

I just ran shake --update; I assume this auto-removes ignore instructions that no longer do anything?

It auto-removes entries from ignore but not ignoreImport or ignoreAll. This one might have been on those other lists.

@eric-wieser
Copy link
Member Author

Can we leave the shake-reconfiguration to a follow-up?

@adomani
Copy link
Collaborator

adomani commented Apr 5, 2024

Yes, I think that it does not make much sense to edit these noshake files manually.

Thanks!

maintainer merge

Copy link

github-actions bot commented Apr 5, 2024

🚀 Pull request has been placed on the maintainer queue by adomani.

Copy link
Contributor

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

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

Thanks!

bors merge

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Apr 5, 2024
mathlib-bors bot pushed a commit that referenced this pull request Apr 5, 2024
I noticed that this doesn't actually use `Std` or in fact any of its other imports.

The downstream fallout is pretty small because `Mathlib.Tactic.Basic` imports `Std` anyway, so this only impacts files not downstream of that.
@mathlib-bors
Copy link

mathlib-bors bot commented Apr 5, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore(Tactic/TypeStar): reduce imports [Merged by Bors] - chore(Tactic/TypeStar): reduce imports Apr 5, 2024
@mathlib-bors mathlib-bors bot closed this Apr 5, 2024
@mathlib-bors mathlib-bors bot deleted the eric-wieser/reduce-imports branch April 5, 2024 13:39
Louddy pushed a commit that referenced this pull request Apr 15, 2024
I noticed that this doesn't actually use `Std` or in fact any of its other imports.

The downstream fallout is pretty small because `Mathlib.Tactic.Basic` imports `Std` anyway, so this only impacts files not downstream of that.
atarnoam pushed a commit that referenced this pull request Apr 16, 2024
I noticed that this doesn't actually use `Std` or in fact any of its other imports.

The downstream fallout is pretty small because `Mathlib.Tactic.Basic` imports `Std` anyway, so this only impacts files not downstream of that.
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
I noticed that this doesn't actually use `Std` or in fact any of its other imports.

The downstream fallout is pretty small because `Mathlib.Tactic.Basic` imports `Std` anyway, so this only impacts files not downstream of that.
callesonne pushed a commit that referenced this pull request Apr 22, 2024
I noticed that this doesn't actually use `Std` or in fact any of its other imports.

The downstream fallout is pretty small because `Mathlib.Tactic.Basic` imports `Std` anyway, so this only impacts files not downstream of that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy < 20s of review time. See the lifecycle page for guidelines. maintainer-merge ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants