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: fix "unnecessary have" lint errors #13680

Closed
wants to merge 2 commits into from

Conversation

llllvvuu
Copy link
Collaborator

@llllvvuu llllvvuu commented Jun 10, 2024


Open in Gitpod

For some reason, these only go off on my Lean PR testing branch.

Some of the ones that went off there were genuinely unnecessary haves.
Others were "unnecessary" for the def, but necessary for
`decreasing_tactic`. Others were declarations getting flagged for
depending on declarations that had unnecessary haves.

I don't know which things are bugs in the linter, which things are bugs
in my Lean patch, and which things are bugs related to what my Lean
patch fixed.
@llllvvuu llllvvuu added awaiting-review easy < 20s of review time. See the lifecycle page for guidelines. awaiting-CI labels Jun 10, 2024
Copy link

github-actions bot commented Jun 10, 2024

PR summary

Import changes

No significant changes to the import graph


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/no_lost_declarations.sh short <optional_commit>

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

@llllvvuu llllvvuu changed the title chore: fix unnecessary have lint errors chore: fix "unnecessary have" lint errors Jun 10, 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.

These fixes certainly seem fine to me.
Can you update the PR description, please?

@llllvvuu
Copy link
Collaborator Author

Updated.

Copy link
Collaborator

@Ruben-VandeVelde Ruben-VandeVelde left a comment

Choose a reason for hiding this comment

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

maintainer merge

Copy link

🚀 Pull request has been placed on the maintainer queue by Ruben-VandeVelde.

@fpvandoorn
Copy link
Member

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Jun 10, 2024
mathlib-bors bot pushed a commit that referenced this pull request Jun 10, 2024



Co-authored-by: L Lllvvuu <git@llllvvuu.dev>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jun 10, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: fix "unnecessary have" lint errors [Merged by Bors] - chore: fix "unnecessary have" lint errors Jun 10, 2024
@mathlib-bors mathlib-bors bot closed this Jun 10, 2024
@mathlib-bors mathlib-bors bot deleted the llllvvuu/unnecessary_have_linter branch June 10, 2024 10:33
mathlib-bors bot pushed a commit that referenced this pull request Jun 12, 2024
This linter was silently not doing anything until leanprover/lean4#4410 was fixed, and now it is working so a backlog of warnings needed to be addressed. Some were addressed here: #13680.

The warnings in this PRs are false positives (leanprover-community/batteries#428?), but a workaround is put in place.



Co-authored-by: L Lllvvuu <git@llllvvuu.dev>
AntoineChambert-Loir pushed a commit that referenced this pull request Jun 20, 2024



Co-authored-by: L Lllvvuu <git@llllvvuu.dev>
AntoineChambert-Loir pushed a commit that referenced this pull request Jun 20, 2024
This linter was silently not doing anything until leanprover/lean4#4410 was fixed, and now it is working so a backlog of warnings needed to be addressed. Some were addressed here: #13680.

The warnings in this PRs are false positives (leanprover-community/batteries#428?), but a workaround is put in place.



Co-authored-by: L Lllvvuu <git@llllvvuu.dev>
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.

4 participants