Skip to content

[Merged by Bors] - feat(Data.DFinsupp.Notation): add notation for DFinsupp#10962

Closed
eric-wieser wants to merge 12 commits intomasterfrom
eric-wieser/DFinsupp-notation
Closed

[Merged by Bors] - feat(Data.DFinsupp.Notation): add notation for DFinsupp#10962
eric-wieser wants to merge 12 commits intomasterfrom
eric-wieser/DFinsupp-notation

Conversation

@eric-wieser
Copy link
Copy Markdown
Member

@eric-wieser eric-wieser commented Feb 25, 2024

This just extends the existing notation we already use for Finsupp (#6367).
The two seem to coexist on the same notation without any issue.


Open in Gitpod

This just extends the existing notation we already use for `Finsupp`
@eric-wieser eric-wieser added awaiting-review t-meta Tactics, attributes or user commands awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. labels Feb 25, 2024
@eric-wieser eric-wieser changed the title feat(Data.DFinsupp.Notation): add notation for DFinsupp` feat(Data.DFinsupp.Notation): add notation for DFinsupp Feb 25, 2024
Co-authored-by: Timo Carlin-Burns <timorcb@gmail.com>
@github-actions github-actions bot removed the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Feb 25, 2024
@eric-wieser eric-wieser requested a review from kmill February 26, 2024 21:57
Copy link
Copy Markdown
Contributor

@joneugster joneugster left a comment

Choose a reason for hiding this comment

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

This looks like a very clean and short PR to me.

Hopefully it should be easy for a reviewer to delegate/merge!

Only question I have is whether there is any test-case for Finsupp which ensures the notation for Finsupp takes priority over DFinsupp, and if not, then maybe one could be added.

@eric-wieser
Copy link
Copy Markdown
Member Author

Only question I have is whether there is any test-case for Finsupp which ensures the notation for Finsupp takes priority over DFinsupp, and if not, then maybe one could be added.

Indeed, in the presence of both imports breaks things

@eric-wieser
Copy link
Copy Markdown
Member Author

Indeed, in the presence of both imports breaks things

This is now fixed.

Copy link
Copy Markdown
Contributor

@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.

Thanks, that's very nice!

maintainer merge

@github-actions
Copy link
Copy Markdown

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

@github-actions github-actions bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label May 18, 2024
Copy link
Copy Markdown
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

@ghost ghost added the ready-to-merge This PR has been sent to bors. label May 18, 2024
@ghost ghost removed the awaiting-review label May 18, 2024
mathlib-bors bot pushed a commit that referenced this pull request May 18, 2024
This just extends the existing notation we already use for `Finsupp` (#6367).
The two seem to coexist on the same notation without any issue.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented May 18, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat(Data.DFinsupp.Notation): add notation for DFinsupp [Merged by Bors] - feat(Data.DFinsupp.Notation): add notation for DFinsupp May 18, 2024
@mathlib-bors mathlib-bors bot closed this May 18, 2024
@mathlib-bors mathlib-bors bot deleted the eric-wieser/DFinsupp-notation branch May 18, 2024 17:10
callesonne pushed a commit that referenced this pull request Jun 4, 2024
This just extends the existing notation we already use for `Finsupp` (#6367).
The two seem to coexist on the same notation without any issue.
js2357 pushed a commit that referenced this pull request Jun 18, 2024
This just extends the existing notation we already use for `Finsupp` (#6367).
The two seem to coexist on the same notation without any issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. ready-to-merge This PR has been sent to bors. t-meta Tactics, attributes or user commands

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants