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(Algebra/Ring): remove variables unused in section #17178

Closed
wants to merge 1 commit into from

Conversation

b-mehta
Copy link
Contributor

@b-mehta b-mehta commented Sep 27, 2024

Perhaps we should have a linter for this... @adomani?


Open in Gitpod

@b-mehta b-mehta added the easy < 20s of review time. See the lifecycle page for guidelines. label Sep 27, 2024
Copy link

PR summary 8f2d3164d5

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

No declarations were harmed in the making of this PR! 🐙

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.

@github-actions github-actions bot added the t-algebra Algebra (groups, rings, fields, etc) label Sep 27, 2024
@adomani
Copy link
Collaborator

adomani commented Sep 27, 2024

This seems like a fun linter to write! Environment linters don't know about variables and syntax linters have a hard time persisting information across commands.

I'll try to see if something can be done!

@adomani
Copy link
Collaborator

adomani commented Sep 29, 2024

Bhavik, I wrote a linter and these are the first unused variables found: #17261.

There are many more: some are potential false positives, since the linter is not really able to deal with declarations that use the equation compiler, but a lot of the output are actually unused variables.

@adomani
Copy link
Collaborator

adomani commented Sep 29, 2024

maintainer merge

Copy link

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

mathlib-bors bot pushed a commit that referenced this pull request Sep 30, 2024
Inspired by #17178 and found by a linter.
@kim-em
Copy link
Contributor

kim-em commented Sep 30, 2024

bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Sep 30, 2024
mathlib-bors bot pushed a commit that referenced this pull request Sep 30, 2024
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Sep 30, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore(Algebra/Ring): remove variables unused in section [Merged by Bors] - chore(Algebra/Ring): remove variables unused in section Sep 30, 2024
@mathlib-bors mathlib-bors bot closed this Sep 30, 2024
@mathlib-bors mathlib-bors bot deleted the unused-variables-ring-defs branch September 30, 2024 03:28
chrisflav pushed a commit that referenced this pull request Oct 1, 2024
Inspired by #17178 and found by a linter.
chrisflav pushed a commit that referenced this pull request Oct 1, 2024
fbarroero pushed a commit that referenced this pull request Oct 2, 2024
Inspired by #17178 and found by a linter.
fbarroero pushed a commit that referenced this pull request Oct 2, 2024
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. t-algebra Algebra (groups, rings, fields, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants