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] - feat(algebra/triv_sq_zero_ext): trivial square-zero extension #5109

Closed
wants to merge 7 commits into from

Conversation

kckennylau
Copy link
Collaborator


@kckennylau kckennylau added the awaiting-review The author would like community review of the PR label Nov 25, 2020
@jcommelin
Copy link
Member

jcommelin commented Nov 25, 2020

I'm afraid that the abbreviation tsze will not really scale. Would you mind spelling it out completely?
It's ok to use local notation to keep this file short and readable.
But outside this file, I think tsze would harm readability.

Apart from that LGTM

@kckennylau
Copy link
Collaborator Author

so trivial_square_zero_extension? won't this be a bit long?

@jcommelin
Copy link
Member

How about a compromise? triv_sq_zero_ext? Most people can guess what that means, where as with tsze they cannot.
It's longer, hence I suggest that people use local notation if they need this a lot. But if you don't need it a lot, then it will be very helpful to know what this tsze actually is. Of course you can hover (if you are in an editor), but that's not something I want to rely on.
So I would default to longer and readable names, and then use local notation to make it shorter.

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

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Nov 25, 2020
@jcommelin
Copy link
Member

bors r-

@bors
Copy link

bors bot commented Nov 25, 2020

Canceled.

@jcommelin
Copy link
Member

Oops, @kckennylau would you mind changing the filename as well?

@jcommelin jcommelin changed the title feat(algebra/tsze): trivial square-zero extension feat(algebra/triv_sq_zero_ext): trivial square-zero extension Nov 25, 2020
@jcommelin jcommelin added awaiting-author A reviewer has asked the author a question or requested changes and removed ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) labels Nov 25, 2020
src/algebra/tsze.lean Outdated Show resolved Hide resolved
src/algebra/tsze.lean Outdated Show resolved Hide resolved
src/algebra/tsze.lean Outdated Show resolved Hide resolved
src/algebra/tsze.lean Outdated Show resolved Hide resolved
src/algebra/tsze.lean Outdated Show resolved Hide resolved
src/algebra/tsze.lean Outdated Show resolved Hide resolved
kckennylau and others added 2 commits November 25, 2020 22:07
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
src/algebra/tsze.lean Outdated Show resolved Hide resolved
src/algebra/triv_sq_zero_ext.lean Outdated Show resolved Hide resolved
src/algebra/triv_sq_zero_ext.lean Outdated Show resolved Hide resolved
src/algebra/triv_sq_zero_ext.lean Outdated Show resolved Hide resolved
src/algebra/triv_sq_zero_ext.lean Show resolved Hide resolved
src/algebra/triv_sq_zero_ext.lean Outdated Show resolved Hide resolved
src/algebra/triv_sq_zero_ext.lean Outdated Show resolved Hide resolved
src/algebra/triv_sq_zero_ext.lean Outdated Show resolved Hide resolved
src/algebra/triv_sq_zero_ext.lean Outdated Show resolved Hide resolved
src/algebra/triv_sq_zero_ext.lean Outdated Show resolved Hide resolved
Co-authored-by: Johan Commelin <johan@commelin.net>
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

@github-actions github-actions bot added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Nov 25, 2020
@github-actions github-actions bot removed the awaiting-author A reviewer has asked the author a question or requested changes label Nov 25, 2020
@bors
Copy link

bors bot commented Nov 25, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(algebra/triv_sq_zero_ext): trivial square-zero extension [Merged by Bors] - feat(algebra/triv_sq_zero_ext): trivial square-zero extension Nov 25, 2020
@bors bors bot closed this Nov 25, 2020
@bors bors bot deleted the tsze branch November 25, 2020 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants