-
Notifications
You must be signed in to change notification settings - Fork 839
[Merged by Bors] - style(misc): fix whitespace #30691
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
Conversation
Extracted from leanprover-community#30658. Found by extending the commandStart linter to proof bodies.
PR summary d9d1cc1012Import changes for modified filesNo significant changes to the import graph Import changes for all files
Declarations diff
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 No changes to technical debt.You can run this locally as
|
| mem_mul m₁ hm₁ i x₁ _hx₁ ih₁ => | ||
obtain ⟨v₁, rfl⟩ := hm₁ | ||
-- this is the first interesting goal | ||
-- This is the first interesting goal. |
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.
Why are we changing the comment here (and below)? The comment above is also not capitalized.
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 can only speculate about Damiano's intentions. I personally think this is better style, but I'm happy to revert this change if you prefer (or to also change the comment a few lines up).
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 was simply following Michael's convention to properly capitalise and add punctuation for fully-formed sentences, but only on places that I noticed while looking at the code. I don't have any special attachment to the non-linter-enforced changes in the PR.
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 feel like that is convention is more relevant for doc-strings. For inline comments, I think there can be more freedom.
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 have tweaked the comment above, and have a slight preference for this way. If you feel strongly enough, I'll revert that part.
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.
Thanks for the careful look!
| mem_mul m₁ hm₁ i x₁ _hx₁ ih₁ => | ||
obtain ⟨v₁, rfl⟩ := hm₁ | ||
-- this is the first interesting goal | ||
-- This is the first interesting goal. |
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 can only speculate about Damiano's intentions. I personally think this is better style, but I'm happy to revert this change if you prefer (or to also change the comment a few lines up).
Mathlib/RepresentationTheory/Homological/GroupCohomology/Basic.lean
Outdated
Show resolved
Hide resolved
Mathlib/RepresentationTheory/Homological/GroupHomology/Basic.lean
Outdated
Show resolved
Hide resolved
Barring further objections, I'll merge this in 24 hours. |
Mathlib/RepresentationTheory/Homological/GroupHomology/Basic.lean
Outdated
Show resolved
Hide resolved
Let's merge this: |
Thanks for the careful reviews, again! |
Extracted from #30658. Found by extending the commandStart linter to proof bodies. Co-authored-by: adomani <adomani@gmail.com>
This PR was included in a batch that was canceled, it will be automatically retried |
Extracted from #30658. Found by extending the commandStart linter to proof bodies. Co-authored-by: adomani <adomani@gmail.com>
Pull request successfully merged into master. Build succeeded: |
Extracted from #30658. Found by extending the commandStart linter to proof bodies.