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.

Sign up for GitHub

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

Jump to bottom

feat: make registerLabelAttr consistent with registerTagAttribute #3699

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

feat: make registerLabelAttr consistent with registerTagAttribute #3699

wants to merge 1 commit into from
+12 −7

Conversation

thorimur
Copy link
Contributor

@thorimur thorimur commented Mar 16, 2024 •
edited
Loading

This PR modifies the recently-upstreamed LabelAttributes to be consistent with TagAttributes. Specifically, we modify registerLabelAttr to

  • allow a validate argument
  • allow an AttributeApplicationTime argument
  • change the default applicationTime to .afterTypeChecking
  • use the same argument order as registerTagAttribute

This also modifies some docstrings to point users to these features.

@thorimur thorimur force-pushed the label-attribute-tag-consistency branch 2 times, most recently from c6a90bf to 5fc348a Compare March 16, 2024 21:46
@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 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 16, 2024
@leanprover-community-mathlib4-bot
Update lean-toolchain for testing leanprover/lean4#3699
34c477c
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 16, 2024
@leanprover-community-mathlib4-bot
Update lean-toolchain for testing leanprover/lean4#3699
4cf715b
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Mar 16, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Mar 16, 2024 •
edited
Loading

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 16, 2024
@leanprover-community-mathlib4-bot
Trigger CI for leanprover/lean4#3699
5cc8402
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 16, 2024
@leanprover-community-mathlib4-bot
Trigger CI for leanprover/lean4#3699
1e373ff
@thorimur thorimur force-pushed the label-attribute-tag-consistency branch from 5fc348a to 27541c9 Compare March 17, 2024 00:08
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 17, 2024
@leanprover-community-mathlib4-bot
Trigger CI for leanprover/lean4#3699
1093668
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 17, 2024
@leanprover-community-mathlib4-bot
Trigger CI for leanprover/lean4#3699
31bed18
thorimur pushed a commit to leanprover-community/mathlib4 that referenced this pull request Mar 17, 2024
@leanprover-community-mathlib4-bot @thorimur
Update lean-toolchain for testing leanprover/lean4#3699
f621499
thorimur pushed a commit to leanprover-community/mathlib4 that referenced this pull request Mar 17, 2024
@leanprover-community-mathlib4-bot @thorimur
Trigger CI for leanprover/lean4#3699
d292e94
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added full-ci builds-mathlib CI has verified that Mathlib builds against this PR and removed breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan labels Mar 17, 2024
@kim-em
Copy link
Collaborator

kim-em commented May 10, 2024

Just confirming that this is still a draft. The changes do seem nice to have!

@Kha Kha removed the full-ci label May 24, 2024
@thorimur
Copy link
Contributor Author

@semorrison Apologies, I've been inactive for the past few months! :) If this is still desired I'm happy to finalize it and make it a real PR.

@thorimur thorimur force-pushed the label-attribute-tag-consistency branch from 27541c9 to 508b317 Compare January 14, 2025 22:16
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 9dbe5e6f9ce6bc686541218f62ff6b8063dfc4dc --onto e9bd9807ef7a983365df9ac55d35040d0b2d5ef2. (2025-01-14 22:45:17)

@thorimur
feat: make registerLabelAttr consistent with registerTagAttribute
d1d788b 
* allow `validate` argument
* allow `AttributeApplicationTime` argument
* change default `applicationTime` to `.afterTypeChecking`
@thorimur thorimur force-pushed the label-attribute-tag-consistency branch from 508b317 to d1d788b Compare January 14, 2025 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@thorimur @leanprover-community-mathlib4-bot @kim-em @leanprover-community-bot @Kha