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

Make Country of Birth only show modern-day countries and countries at author birthdate in reading stats #6550

Merged
merged 4 commits into from
May 23, 2022

Conversation

cdrini
Copy link
Collaborator

@cdrini cdrini commented May 13, 2022

SQUASH ME

Previously, the "Country of Birth" section of reading log stats would show historical countries that had no real relation to the author (eg it might show "Ancient Greece" for someone born in modern day Greece). Change it so that it now only counts modern day countries (which are probably more recognizable to most folks) as well as the historical country that existed at the time of the author's birth (technically more correct).

Technical

Testing

Compare the reading stats for "Country of Birth" on staging vs testing.

Screenshot

Here's how mine changed:
image

Stakeholders

@bicolino34

@cdrini cdrini changed the title Make Country of Birth only show modern-day countries in reading stats Make Country of Birth only show modern-day countries and countries at author birthdate in reading stats May 13, 2022
@cdrini cdrini added the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label May 13, 2022
@cdrini cdrini marked this pull request as ready for review May 13, 2022 18:44
@mekarpeles mekarpeles self-assigned this May 16, 2022
@mekarpeles mekarpeles merged commit 0d3263f into master May 23, 2022
@mekarpeles mekarpeles deleted the cdrini-patch-2 branch May 23, 2022 19:18
@jimchamp jimchamp removed the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Apr 2, 2024
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.

3 participants