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: avoid some unused variables #11594
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just one nit, which is not important.
simp_rw [SubmonoidClass.coe_pow, Units.val_pow_eq_pow_val, OneMemClass.coe_one, | ||
Units.val_one, h] | ||
· exact h | ||
refine hzero.resolve_left fun h => ha ?_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use "\maps" instead of "=>" while you're at it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Not official or anything, but my preference is against "while you're at it" suggestions. They really do, in my opinion, add friction to getting stuff done.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand this point!
My point was this is a new proof written - so reviewing doesn't become any harder this way. Authoring it does; thanks for pointing this out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me add: I can make sure to, in the future,
- make such comments directly as suggestions (minimizing the overhead for authors and reviewers)
- limit these comments to new proofs, or PRs cleaning up miscellaneous things, where such a change is in scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bors d+
✌️ Ruben-VandeVelde can now approve this pull request. To approve and merge a pull request, simply reply with |
bors merge |
These will be caught by the linter in a future lean version.
Build failed: |
I think #11609 fixes the build error here. Feel free to cherry-pick. |
bors merge |
These will be caught by the linter in a future lean version.
Pull request successfully merged into master. Build succeeded: |
These will be caught by the linter in a future lean version.
These will be caught by the linter in a future lean version.
These will be caught by the linter in a future lean version.
These will be caught by the linter in a future lean version.
These will be caught by the linter in a future lean version.
These will be caught by the linter in a future lean version.