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

kernel: tweak error messages to say "an object" #3691

Merged

Conversation

fingolfin
Copy link
Member

... instead of "a object", and similar.

This makes some progress on issue #3400 and is otherwise about the least import PR we had in a long time, I guess ;-).

@fingolfin fingolfin added topic: kernel release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Oct 6, 2019
@ChrisJefferson
Copy link
Contributor

I understand the reason someone would want this, but it's going to break the tests of lots of packages, and make it hard to easily write tests with error messages which work in versions of GAP both before and after this patch.

I don't have a good idea on how to fix this. A configuration option feels overkill.

... instead of "a object", and similar.
@fingolfin
Copy link
Member Author

I actually do not believe this will break a single package test:

  1. These errors concern validation of GAP kernel functions; no package test ought to trigger any of those.
  2. To verify this, I grepped through all deposited packages plus a ton of package git repositories I have on my computer, using rg -F '(not a ', and as far as I could tell, none of them trigger any of the affected kernel error messages. There were only a few hits anyway; those in .tst files either were inside comments (for atlasrep), or came from package functions (for datastructures).
  3. In GAP 4.11, we completely overhauled these errors (by adding those Require* macros, and also by changing many TNAMs); it's very likely that those changes would have broken packages which this PR here would break. But none were broken, which seems to support my point 2.

@ChrisJefferson
Copy link
Contributor

Yep, I was too cautious.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 84.439% when pulling b3e0730 on fingolfin:mh/error-article-inflection into 3d9c302 on gap-system:master.

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: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants