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

Revert some Semigroups-related downrankings of Matrix methods #3674

Merged
merged 1 commit into from Sep 25, 2019

Conversation

wilfwilson
Copy link
Member

@wilfwilson wilfwilson commented Sep 23, 2019

Three downrankings of Matrix methods were added in advance of GAP 4.10, along with one for BaseDomain, in order to make sure that GAP 4.10 did not break the Semigroups package. This was supposed to be temporary, and #2758 exists to revert this change.

In fact, only one of these downrankings is required to keep Semigroups happy. This commit reverts the others. Once it is merged, I will update #2758 accordingly.

At some point (separately from this PR) I will try to see about removing the remaining downranking of a Matrix method.

@wilfwilson wilfwilson added topic: library release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Sep 23, 2019
@coveralls
Copy link

coveralls commented Sep 23, 2019

Coverage Status

Coverage increased (+0.0005%) to 84.443% when pulling 47d259f on remove-downranking into fcd1f51 on master.

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Looks good to me. I did not verify how this affects Semigroups or anything else, though -- i.e., I am trusting @wilfwilson on this (and am happy to do so).

I guess this could/should be backported to stable-4.11, right?

@wilfwilson wilfwilson added the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Sep 24, 2019
@wilfwilson
Copy link
Member Author

There is an additional FIXME that I didn't consider - I'll check it out shortly.

Four downrankings were added in advance of GAP 4.10, to make
sure that GAP did not break the Semigroups package, but only
one of them was actually required. This commit reverts the
other three.
@wilfwilson wilfwilson removed the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Sep 24, 2019
@wilfwilson
Copy link
Member Author

I've also reverted the BaseDomain downranking that also existed (thanks @PaulaHaehndel for pointing it out to me). Doing so doesn't seem to affect the Semigroups package, so I don't think there can be any harm in removing it.

@fingolfin As for backporting, I don't mind. These downrankings didn't seem to be having any consequences, so I don't know whether backporting would make any noticeable difference to anything. Please add the backport tag if you think it's appropriate.

@wilfwilson
Copy link
Member Author

Just to clarify, I ran the GAP testinstall, teststandard, testbugfix test files with this branch of GAP and with the latest released version of the Semigroups package loaded, and I also ran the Semigroups package tests in this version of GAP. Everything passed as normal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants