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

chore: remove unused deriving handler argument syntax #5265

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

david-christiansen
Copy link
Contributor

As far as I can tell, the ability to pass a structure instance to a deriving handler is not actually used in practice. It didn't seem to be used in the test suite, at least.

Do we want to remove this, or do we want to use and document it? This PR removes it, but that's not something I feel strongly about - but seeing if it breaks Mathlib is a useful data point.

As far as I can tell, the ability to pass a structure instance to a
deriving handler is not actually used in practice.

Do we want to remove this, or do we want to use and document it?
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Sep 5, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 5, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 5, 2024
@leanprover-community-bot leanprover-community-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Sep 5, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 9, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 9, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

@david-christiansen
Copy link
Contributor Author

From what I can read here, the Mathlib breakage is only due to a deprecation warning being issued, and not that it doesn't compile, right?

If so, I take that as evidence that this syntax is indeed not used and can be removed.

@kim-em
Copy link
Collaborator

kim-em commented Sep 26, 2024

From what I can read here, the Mathlib breakage is only due to a deprecation warning being issued, and not that it doesn't compile, right?

Right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants