review: suggested changes for #27021#1
Merged
grunweg merged 47 commits intoOct 6, 2025
Merged
Conversation
thorimur
force-pushed
the
diffgeo-custom-elaborators-review
branch
from
8d1726d to
f363078
Compare
… make descriptions into docstrings
…; search local instances for classes instead of whole context
thorimur
force-pushed
the
diffgeo-custom-elaborators-review
branch
from
8d0cd7c to
11f0795
Compare
… different strategies; refactor `findModel` to make this possible. * Note that the control flow changes slightly as a result of this commit. `TotalSpace`s will not cause top-level errors when both `TotalSpace` strategies fail; rather, the remaining strategies will be tried just in case. The given error message is demoted to a trace message. Better error messages for `findModel` can be provided in subsequent PRs.
thorimur
force-pushed
the
diffgeo-custom-elaborators-review
branch
2 times, most recently
from
bb78d1d to
11fb2b9
Compare
…'t use `match_expr`
thorimur
force-pushed
the
diffgeo-custom-elaborators-review
branch
from
11fb2b9 to
d35bb01
Compare
thorimur
force-pushed
the
diffgeo-custom-elaborators-review
branch
from
d35bb01 to
30c018a
Compare
thorimur
force-pushed
the
diffgeo-custom-elaborators-review
branch
from
30c018a to
1976265
Compare
Owner
|
Thanks a lot for this thorough and well-explained clean-up. I took a look at everything, and just have a few minor questions. (Once these are answered, I will probably just merge your PR.)
|
Owner
|
@PatrickMassot In case you want to carefully double-check the logic and have way too much time at your hands, a0587df and d61a7ed could be worth a look. I'm willing to trust Thomas and the tests we have to find any issues. |
Owner
|
Actually: let me merge this PR already and keep track of the remaining commits on the main PR. |
grunweg
merged commit c51a943
into
grunweg:diffgeo-custom-elaborators
3 of 4 checks passed
Owner
|
Thanks again! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Suggested changes for mathlib4#27021. These spanned many lines, which made the usual GitHub interface awkward.
The suggestions are explained in a review.