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

Correct bug in DirectFactorsOfGroup #566

Merged

Conversation

hungaborhorvath
Copy link
Contributor

Stupid mistake: instead of (A or B) and C, there was A or B and C,
which evaluated true instead of false for A=true and C=false.

This was a bug was introduced in #379, and reported by @alex-konovalov there after the merge.

Stupid mistake: instead of "(A or B) and C", there was "A or B and C",
which evaluated true instead of false for A=true and C=false.
@markuspf
Copy link
Member

markuspf commented Feb 1, 2016

This looks good to me, and I'd be keen to merge soon, since this fixes a bug that causes a wrong result.

@olexandr-konovalov
Copy link
Member

To me too, so I am merging this now.

The fix in the test teaches a lesson that you that when the test created from the output is passing, that does not always mean that that output is correct :)

olexandr-konovalov pushed a commit that referenced this pull request Feb 1, 2016
Correct bug in DirectFactorsOfGroup
@olexandr-konovalov olexandr-konovalov merged commit 8d2a6c7 into gap-system:master Feb 1, 2016
@hungaborhorvath
Copy link
Contributor Author

@alex-konovalov Fair enough, duly noted.

@hungaborhorvath hungaborhorvath deleted the DirectFactorBug branch February 1, 2016 23:08
@olexandr-konovalov olexandr-konovalov added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Jan 20, 2018
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants