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] - refactor(data/polynomial/ring_division): Generalize irreducible_of_monic to no_zero_divisors #17696

Closed
wants to merge 7 commits into from

Conversation

tb65536
Copy link
Collaborator

@tb65536 tb65536 commented Nov 23, 2022

This PR generalizes irreducible_of_monic to no_zero_divisors, using some of the work from @alreadydone's PR #17664.

Co-Authored-By: Junyan Xu junyanxumath@gmail.com


Open in Gitpod

@tb65536 tb65536 added awaiting-review The author would like community review of the PR t-algebra Algebra (groups, rings, fields etc) labels Nov 23, 2022
alreadydone added a commit that referenced this pull request Nov 24, 2022
@Vierkantor Vierkantor self-assigned this Nov 24, 2022
Copy link
Collaborator

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! I went ahead and added a Co-Authored-By line in the PR description so Git is also aware of @alreadydone's contributions.

@Vierkantor
Copy link
Collaborator

bors d+

@bors
Copy link

bors bot commented Nov 24, 2022

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

@github-actions github-actions bot added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Nov 24, 2022
@alreadydone
Copy link
Collaborator

I think it's better to close this and merge my PR instead, otherwise there will be conflicts, since I didn't merge this PR into my PR.

@Vierkantor
Copy link
Collaborator

That's also fine for me if @tb65536 doesn't mind getting less credit in the Git statistics :)

@tb65536
Copy link
Collaborator Author

tb65536 commented Nov 24, 2022

I think it's better to close this and merge my PR instead, otherwise there will be conflicts, since I didn't merge this PR into my PR.

@alreadydone I would still prefer to just go ahead and merge this, since it will make your PR more focused and easier to review. Yes, there will be some merge conflicts, but they should be easy to resolve since you already incorporated the changes. And then you can either use your results to further golf irreducible_of_monic or use irreducible_of_monic to golf your results.

@tb65536
Copy link
Collaborator Author

tb65536 commented Nov 24, 2022

bors r+

bors bot pushed a commit that referenced this pull request Nov 24, 2022
…onic` to `no_zero_divisors` (#17696)

This PR generalizes `irreducible_of_monic` to `no_zero_divisors`, using some of the work from @alreadydone's PR #17664.

Co-Authored-By: Junyan Xu <junyanxumath@gmail.com>
bors bot pushed a commit that referenced this pull request Nov 24, 2022
…onic` to `no_zero_divisors` (#17696)

This PR generalizes `irreducible_of_monic` to `no_zero_divisors`, using some of the work from @alreadydone's PR #17664.

Co-Authored-By: Junyan Xu <junyanxumath@gmail.com>
@bors
Copy link

bors bot commented Nov 24, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title refactor(data/polynomial/ring_division): Generalize irreducible_of_monic to no_zero_divisors [Merged by Bors] - refactor(data/polynomial/ring_division): Generalize irreducible_of_monic to no_zero_divisors Nov 24, 2022
@bors bors bot closed this Nov 24, 2022
@bors bors bot deleted the tb_irred branch November 24, 2022 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions. t-algebra Algebra (groups, rings, fields etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants