Skip to content

[Merged by Bors] - fix: make to_additive deal with abbrev correctly#15474

Closed
astrainfinita wants to merge 13 commits intomasterfrom
FR_fix_additive_abbrev
Closed

[Merged by Bors] - fix: make to_additive deal with abbrev correctly#15474
astrainfinita wants to merge 13 commits intomasterfrom
FR_fix_additive_abbrev

Conversation

@astrainfinita
Copy link
Copy Markdown
Collaborator

@astrainfinita astrainfinita commented Aug 3, 2024 •
edited
Loading

Open in Gitpod

@astrainfinita astrainfinita added the t-meta Tactics, attributes or user commands label Aug 3, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 3, 2024 •
edited
Loading

PR summary f284dc3f01

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

+ setInlineAttribute

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>

The doc-module for script/declarations_diff.sh contains some details about this script.

This was referenced Aug 3, 2024
Closed
Closed
@astrainfinita astrainfinita added the WIP Work in progress label Aug 3, 2024
@ghost ghost added the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Aug 4, 2024
@astrainfinita
Copy link
Copy Markdown
Collaborator Author

astrainfinita commented Aug 4, 2024

!bench

@leanprover-bot
Copy link
Copy Markdown
Collaborator

leanprover-bot commented Aug 4, 2024

Here are the benchmark results for commit ef36607.
There were significant changes against commit d631146:

  Benchmark                         Metric         Change
  =======================================================
+ ~Mathlib.GroupTheory.CosetCover   instructions   -18.0%

@astrainfinita astrainfinita removed the WIP Work in progress label Aug 5, 2024
@eric-wieser
Copy link
Copy Markdown
Member

eric-wieser commented Aug 5, 2024

Does this change need #15483, or are they really orthogonal?

@ghost ghost added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Aug 5, 2024
@astrainfinita
Copy link
Copy Markdown
Collaborator Author

astrainfinita commented Aug 5, 2024

Making QuotientAddGroup.mk reducible leads to some timeouts.

@mattrobball mattrobball mentioned this pull request Aug 8, 2024
Open
1 task
@mattrobball
Copy link
Copy Markdown
Contributor

mattrobball commented Aug 8, 2024 •
edited
Loading

@FR-vdash-bot please ping me if the changes to to_additive don't "just build" on master now

@ghost ghost removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Aug 8, 2024
@astrainfinita
Copy link
Copy Markdown
Collaborator Author

astrainfinita commented Aug 8, 2024

@mattrobball There are still some timeouts.

@astrainfinita
Copy link
Copy Markdown
Collaborator Author

astrainfinita commented Aug 8, 2024

Sorry, I didn't merge master correctly. 😓

mattrobball
mattrobball reviewed Aug 8, 2024
View reviewed changes
@mattrobball
Copy link
Copy Markdown
Contributor

mattrobball commented Aug 8, 2024

Can you include a test verifying the new functionality? Thanks

@astrainfinita
Copy link
Copy Markdown
Collaborator Author

astrainfinita commented Aug 8, 2024

!bench

@ghost ghost removed the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Aug 8, 2024
@astrainfinita astrainfinita mentioned this pull request Aug 8, 2024
Open
@leanprover-bot
Copy link
Copy Markdown
Collaborator

leanprover-bot commented Aug 8, 2024

Here are the benchmark results for commit 953a02b.
There were no significant changes against commit 2eea559.

@mattrobball
Copy link
Copy Markdown
Contributor

mattrobball commented Aug 8, 2024

bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Aug 8, 2024
mathlib-bors bot pushed a commit that referenced this pull request Aug 8, 2024
@astrainfinita
fix: make to_additive deal with abbrev correctly (#15474)
a951fed
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Aug 8, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title fix: make to_additive deal with abbrev correctly [Merged by Bors] - fix: make to_additive deal with abbrev correctly Aug 8, 2024
@mathlib-bors mathlib-bors bot closed this Aug 8, 2024
@mathlib-bors mathlib-bors bot deleted the FR_fix_additive_abbrev branch August 8, 2024 21:19
astrainfinita added a commit that referenced this pull request Aug 8, 2024
@astrainfinita
revert after #15474
2c67c18
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
@astrainfinita @bjoernkjoshanssen
fix: make to_additive deal with abbrev correctly (#15474)
b19ec92
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
@astrainfinita @bjoernkjoshanssen
fix: make to_additive deal with abbrev correctly (#15474)
b85fe66
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 12, 2024
@astrainfinita @bjoernkjoshanssen
fix: make to_additive deal with abbrev correctly (#15474)
39debb1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattrobball mattrobball mattrobball left review comments

Assignees

No one assigned

Labels

ready-to-merge This PR has been sent to bors. t-meta Tactics, attributes or user commands

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@astrainfinita @leanprover-bot @eric-wieser @mattrobball