Skip to content

[Merged by Bors] - fix(Algebra/DirectSum/Internal): delete some instances#18149

Closed
kbuzzard wants to merge 1 commit intomasterfrom
kbuzzard-delete-ofsubmonoidonsemiring
Closed

[Merged by Bors] - fix(Algebra/DirectSum/Internal): delete some instances#18149
kbuzzard wants to merge 1 commit intomasterfrom
kbuzzard-delete-ofsubmonoidonsemiring

Conversation

@kbuzzard
Copy link
Copy Markdown
Member

See Zulip discussion. One of these instances was breaking user code.


Open in Gitpod

@github-actions
Copy link
Copy Markdown

PR summary c1e3767f59

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

- AddCommGroup.ofSubgroupOnRing
- AddCommMonoid.ofSubmonoidOnSemiring

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.

@mattrobball
Copy link
Copy Markdown
Contributor

!bench

@github-actions github-actions bot added the t-algebra Algebra (groups, rings, fields, etc) label Oct 23, 2024
@riccardobrasca
Copy link
Copy Markdown
Member

!bench

@kbuzzard
Copy link
Copy Markdown
Member Author

heh, you both took the words out of my mouth :-)

@riccardobrasca
Copy link
Copy Markdown
Member

Ops, I didn't see Matthew also benched it

@leanprover-bot
Copy link
Copy Markdown
Collaborator

Here are the benchmark results for commit c1e3767.
There were no significant changes against commit 8a18755.

@github-actions
Copy link
Copy Markdown

File Instructions %
build +5.125⬝10⁹ (+0.0%)
lint -1.4⬝10⁹ (+0.1%)
Mathlib.Algebra.DirectSum.Internal +6.874⬝10⁹ (+16.68%)

@mattrobball
Copy link
Copy Markdown
Contributor

Seems fine to me

@riccardobrasca
Copy link
Copy Markdown
Member

I agree

@fpvandoorn
Copy link
Copy Markdown
Member

Yes, looks good.

bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Oct 23, 2024
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Oct 23, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title fix(Algebra/DirectSum/Internal): delete some instances [Merged by Bors] - fix(Algebra/DirectSum/Internal): delete some instances Oct 23, 2024
@mathlib-bors mathlib-bors bot closed this Oct 23, 2024
@mathlib-bors mathlib-bors bot deleted the kbuzzard-delete-ofsubmonoidonsemiring branch October 23, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors. t-algebra Algebra (groups, rings, fields, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants