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 the no-longer-referenced ofReduceNat axiom (or add tests for it, depending on feedback) #7718

Closed
wants to merge 1 commit into from

Conversation

euprunin
Copy link
Contributor

@euprunin euprunin commented Mar 29, 2025

As far as I can tell, ofReduceNat is no longer referenced or used anywhere in the codebase. A review of the commit history shows that its last reference was removed in 2021 (see 9f88ea8). Local testing also suggests that ofReduceNat is unused.

That said, the presence of an unused axiom is somewhat surprising, and since I’m new to this project there’s a chance I might be missing an indirect usage. If so, please let me know and I’ll either close this PR, or add a proper test for ofReduceNat depending on what is deemed the appropriate course of action.

I’m submitting it as a draft to ensure everything builds in CI across all configurations. The next step will be to confirm that it also builds Mathlib successfully. Update: Seems to build Mathlib without problems.

@euprunin euprunin marked this pull request as draft March 29, 2025 11:12
@euprunin euprunin changed the title draft: add tests for the no-longer-referenced ofReduceNat axiom or remove it, depending on CI results and feedback chore: add tests for the no-longer-referenced ofReduceNat axiom or remove it, depending on CI results and feedback Mar 29, 2025
@euprunin euprunin changed the title chore: add tests for the no-longer-referenced ofReduceNat axiom or remove it, depending on CI results and feedback chore: remove the no-longer-referenced ofReduceNat axiom or add tests for it, depending on CI results and feedback Mar 29, 2025
@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 Mar 29, 2025
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Mar 29, 2025

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 5348ce96326c518a45882f3a38363535a6085b17 --onto 060b2fe46fe728934744ee52271b92ab74cbd5b4. You can force Mathlib CI using the force-mathlib-ci label. (2025-03-29 11:35:34)
  • ✅ Mathlib branch lean-pr-testing-7718 has successfully built against this PR. (2025-03-29 12:54:45) View Log

@euprunin euprunin force-pushed the euprunin_ofreducenat branch from c0a6f22 to 8b427e5 Compare March 29, 2025 11:45
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 29, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 29, 2025
@leanprover-community-bot leanprover-community-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Mar 29, 2025
@euprunin euprunin changed the title chore: remove the no-longer-referenced ofReduceNat axiom or add tests for it, depending on CI results and feedback chore: remove the no-longer-referenced ofReduceNat axiom (or add tests for it, depending on CI results and feedback) Mar 29, 2025
@euprunin euprunin changed the title chore: remove the no-longer-referenced ofReduceNat axiom (or add tests for it, depending on CI results and feedback) chore: remove the no-longer-referenced ofReduceNat axiom (or add tests for it, depending on feedback) Mar 29, 2025
@euprunin
Copy link
Contributor Author

@arthur-adjedj Thanks for the feedback! Are you suggesting we add a test instead? Could you clarify how this axiom is being used indirectly? Thanks!

@euprunin euprunin closed this Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR 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.

2 participants