Skip to content

Conversation

grunweg
Copy link
Collaborator

@grunweg grunweg commented Oct 19, 2025

Extracted from #30658. Found by extending the commandStart linter to proof bodies.


Open in Gitpod

Extracted from leanprover-community#30658. Found by extending the commandStart linter to proof bodies.
@github-actions
Copy link

github-actions bot commented Oct 19, 2025

PR summary d9d1cc1012

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

+ instance (priority := 100) small_succ (α : Type v) : Small.{v + 1} α
- instance (priority := 100) small_succ (α : Type v) : Small.{v+1} α

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.


No changes to technical debt.

You can run this locally as

./scripts/technical-debt-metrics.sh pr_summary
  • The relative value is the weighted sum of the differences with weight given by the inverse of the current value of the statistic.
  • The absolute value is the relative value divided by the total sum of the inverses of the current values (i.e. the weighted average of the differences).

| mem_mul m₁ hm₁ i x₁ _hx₁ ih₁ =>
obtain ⟨v₁, rfl⟩ := hm₁
-- this is the first interesting goal
-- This is the first interesting goal.
Copy link
Collaborator

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.

Copy link
Collaborator Author

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).

Copy link
Collaborator

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.

Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

@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.

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.
Copy link
Collaborator Author

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).

@grunweg grunweg added the awaiting-author A reviewer has asked the author a question or requested changes. label Oct 21, 2025
@grunweg grunweg removed the awaiting-author A reviewer has asked the author a question or requested changes. label Oct 21, 2025
@grunweg
Copy link
Collaborator Author

grunweg commented Oct 21, 2025

Barring further objections, I'll merge this in 24 hours.

@grunweg
Copy link
Collaborator Author

grunweg commented Oct 22, 2025

Let's merge this:
bors merge

@grunweg
Copy link
Collaborator Author

grunweg commented Oct 22, 2025

Thanks for the careful reviews, again!

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the ready-to-merge This PR has been sent to bors. label Oct 22, 2025
mathlib-bors bot pushed a commit that referenced this pull request Oct 22, 2025
Extracted from #30658. Found by extending the commandStart linter to proof bodies.

Co-authored-by: adomani <adomani@gmail.com>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Oct 22, 2025

This PR was included in a batch that was canceled, it will be automatically retried

mathlib-bors bot pushed a commit that referenced this pull request Oct 23, 2025
Extracted from #30658. Found by extending the commandStart linter to proof bodies.

Co-authored-by: adomani <adomani@gmail.com>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Oct 23, 2025

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title style(misc): fix whitespace [Merged by Bors] - style(misc): fix whitespace Oct 23, 2025
@mathlib-bors mathlib-bors bot closed this Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants