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

Accommodate correctly pluralized ViewStrings in GAP #19

Merged
merged 1 commit into from
May 20, 2021

Conversation

wilfwilson
Copy link
Member

In a future version of GAP, some nouns will be correctly pluralised to match their number. For example, the ViewString for CyclicGroup(3); will change from <pc group of size 3 with 1 generators> to <pc group of size 3 with 1 generator>. (See gap-system/gap#3992 and gap-system/gap#4050 for more context.)

In this PR, I am trying maintain the backwards and forwards compatibility of this package with GAP. I have done this by suppressing the incorrectly spelled output in tests, and adopting the new output in the .tex files (which I assume are not run through any automated tests?)

Please let me know if this is sufficient - perhaps it was actually important to keep some of the output that I have now suppressed?

@codecov
Copy link

codecov bot commented Oct 21, 2020

Codecov Report

Merging #19 (4f59f06) into master (3df23a1) will not change coverage.
The diff coverage is n/a.

❗ Current head 4f59f06 differs from pull request most recent head 6fa4502. Consider uploading reports for the commit 6fa4502 to get more accurate results

@@           Coverage Diff           @@
##           master      #19   +/-   ##
=======================================
  Coverage   95.98%   95.98%           
=======================================
  Files          62       62           
  Lines       83332    83332           
=======================================
  Hits        79990    79990           
  Misses       3342     3342           

Copy link
Member

@olexandr-konovalov olexandr-konovalov left a comment

Choose a reason for hiding this comment

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

To me, looks sensible - tests will pass, it's ready to be released with the new version of GAP, and it a release happens earlier, one could tolerate slight discrepancy in the manual examples.

@wilfwilson wilfwilson merged commit 572c84d into gap-packages:master May 20, 2021
@wilfwilson wilfwilson deleted the pluralize branch May 20, 2021 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants