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

[Merged by Bors] - chore(Mathlib/Tactic/ToAdditive/Frontend): isolate the to_additive attribute implementation in a new file Tactic.ToAdditive.Frontend (2/2) #14750

Closed
wants to merge 5 commits into from

Conversation

Komyyy
Copy link
Collaborator

@Komyyy Komyyy commented Jul 15, 2024

Ideally, we want to move the attribute [to_additive] commands in Init.ZeroOne into Tactic.ToAdditive without any other changes.
However, the to_additive attribute is initialized in the file and cannot be included in the same file.
So it would be necessary to isolate the original to_additive attribute implementation in a new file Tactic.ToAdditive.Frontend, and create an new file Tactic.ToAdditive as a placeholder for the attribute [to_additive] commands in Init.ZeroOne.
But if we put these two changes in the same PR, git cannot recognizes that Tactic.ToAdditive is renamed, so we split the PR.


Open in Gitpod

@Komyyy Komyyy added awaiting-CI tech debt Tracking cross-cutting technical debt, see e.g. the "Technical debt counters" stream on zulip labels Jul 15, 2024
Copy link

github-actions bot commented Jul 15, 2024

PR summary 11a78505f3

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference
Too many changes (4290)!

Declarations diff

No declarations were harmed in the making of this PR! 🐙

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>

@Komyyy Komyyy changed the title chore(Mathlib/Init/ZeroOne): better home for @[to_additive] attrs in Mathlib.Init.ZeroOne chore(Mathlib/Init/ZeroOne): better home for attribute @[to_additive] commands in Mathlib.Init.ZeroOne Jul 15, 2024
@Komyyy Komyyy requested a review from jcommelin July 15, 2024 10:59
@jcommelin
Copy link
Member

Dear @Komyyy I still don't understand why the file Tactic/ToAdditive needs to be renamed in the same PR that touches Mathlib/Init/ZeroOne. I would like to have a PR that does only the changes to Tactic/ToAdditive.

@jcommelin jcommelin added the awaiting-author A reviewer has asked the author a question or requested changes label Jul 15, 2024
@Komyyy
Copy link
Collaborator Author

Komyyy commented Jul 15, 2024

@jcommelin
Ideally, the attribute [to_additive] commands in Init.ZeroOne should be moved into Tactic.ToAdditive without any other changes.
However, the to_additive attribute is initialized in the file and cannot be included in the same file. So I thought it would be necessary to isolate the original to_additive attribute implementation in a new file Tactic.ToAdditive.Frontend in the same PR as the PR to which the commands in Init.ZeroOne are moved.

@jcommelin
Copy link
Member

I think you can do the "isolation" part before moving the commands in Init.ZeroOne. Or am I missing something?

@Komyyy Komyyy changed the title chore(Mathlib/Init/ZeroOne): better home for attribute @[to_additive] commands in Mathlib.Init.ZeroOne chore(Mathlib/Tactic/ToAdditive/Frontend): isolate the to_additive attribute implementation in a new file Tactic.ToAdditive.Frontend Jul 15, 2024
@Komyyy
Copy link
Collaborator Author

Komyyy commented Jul 15, 2024

@jcommelin
I made this PR do only isolation part and moved the part moving the commands in Init.ZeroOne to #14642.

@Komyyy Komyyy added awaiting-CI and removed awaiting-author A reviewer has asked the author a question or requested changes labels Jul 15, 2024
@jcommelin
Copy link
Member

Thanks. This is now an atomic change.
I assume it will pass CI, but let's wait to be sure:

bors d+

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jul 15, 2024

✌️ Komyyy can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@Komyyy
Copy link
Collaborator Author

Komyyy commented Jul 15, 2024

bors r+

mathlib-bors bot pushed a commit that referenced this pull request Jul 15, 2024
…attribute implementation in a new file `Tactic.ToAdditive.Frontend` (#14750)

This PR is needed to move `attribute [to_additive]` commands in `Mathlib.Init.ZeroOne`.
@Komyyy
Copy link
Collaborator Author

Komyyy commented Jul 15, 2024

bors r-

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jul 15, 2024

Canceled.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jul 15, 2024
@YaelDillies YaelDillies self-assigned this Jul 16, 2024
@Komyyy Komyyy removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jul 17, 2024
@Komyyy Komyyy changed the title chore(Mathlib/Tactic/ToAdditive/Frontend): isolate the to_additive attribute implementation in a new file Tactic.ToAdditive.Frontend chore(Mathlib/Tactic/ToAdditive/Frontend): isolate the to_additive attribute implementation in a new file Tactic.ToAdditive.Frontend (2/2) Jul 17, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added blocked-by-other-PR This PR depends on another PR to Mathlib (this label is automatically managed by a bot) and removed blocked-by-other-PR This PR depends on another PR to Mathlib (this label is automatically managed by a bot) labels Jul 17, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) and removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Jul 17, 2024
@Komyyy Komyyy force-pushed the Komyyy/Mathlib.Init.ZeroOne branch from f231146 to 11a7850 Compare July 17, 2024 08:50
Copy link
Collaborator

@YaelDillies YaelDillies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maintainer merge

Copy link

🚀 Pull request has been placed on the maintainer queue by YaelDillies.

Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 🎉

bors merge

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the ready-to-merge This PR has been sent to bors. label Jul 17, 2024
mathlib-bors bot pushed a commit that referenced this pull request Jul 17, 2024
…attribute implementation in a new file `Tactic.ToAdditive.Frontend` (2/2) (#14750)

Ideally, we want to move the `attribute [to_additive]` commands in `Init.ZeroOne` into `Tactic.ToAdditive` without any other changes.
However, the `to_additive` attribute is initialized in the file and cannot be included in the same file.
So it would be necessary to isolate the original `to_additive` attribute implementation in a new file `Tactic.ToAdditive.Frontend`, and create an new file `Tactic.ToAdditive` as a placeholder for the `attribute [to_additive]` commands in `Init.ZeroOne`.
But if we put these two changes in the same PR, git cannot recognizes that `Tactic.ToAdditive` is renamed, so we split the PR.
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jul 17, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore(Mathlib/Tactic/ToAdditive/Frontend): isolate the to_additive attribute implementation in a new file Tactic.ToAdditive.Frontend (2/2) [Merged by Bors] - chore(Mathlib/Tactic/ToAdditive/Frontend): isolate the to_additive attribute implementation in a new file Tactic.ToAdditive.Frontend (2/2) Jul 17, 2024
@mathlib-bors mathlib-bors bot closed this Jul 17, 2024
@mathlib-bors mathlib-bors bot deleted the Komyyy/Mathlib.Init.ZeroOne branch July 17, 2024 10:07
@adomani adomani mentioned this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated maintainer-merge ready-to-merge This PR has been sent to bors. tech debt Tracking cross-cutting technical debt, see e.g. the "Technical debt counters" stream on zulip
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants