Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Update map baselines and fix failing font download #7396

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

Merged
emilykl merged 2 commits into master from update-map-baselines
Mar 31, 2025

Conversation

Copy link
Contributor

@emilykl emilykl commented Mar 27, 2025
edited
Loading

Image tests on some of the map traces are failing, apparently because Carto made some minor changes to their basemaps. These changes don't seem to be related to anything in our own codebase.

This PR updates the baseline images to match the new basemaps.

This PR also updates the font download URL for Gravitas One in the CI script, which changed due to expo/google-fonts#136. This fixes the other failing image tests. Also updates the CI script such that an error is raised if the font file fails to download, which will make future debugging much easier.

archmoj reacted with heart emoji
Copy link
Contributor Author

emilykl commented Mar 27, 2025

I've looked at all the image diffs; the only significant change is that (apparently) ocean names are no longer displayed on the basemaps. All of the other differences are minor or invisible.

archmoj reacted with heart emoji

Copy link
Collaborator

@marthacryan marthacryan left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

archmoj reacted with heart emoji
Copy link
Contributor

archmoj commented Mar 29, 2025

Wondering if other failing image tests

 "various_geo_projections",
 "smith_template",
 "geo_text_chart_arrays",
 "font-wishlist",
 "canada_geo_projections"

are related to #7359?

archmoj reacted with heart emoji

@gvwilson gvwilson added P1 needed for current cycle fix fixes something broken labels Mar 31, 2025
@emilykl emilykl changed the title (削除) update map baselines (削除ここまで) (追記) Update map baselines and fix failing font download (追記ここまで) Mar 31, 2025
Copy link
Contributor Author

emilykl commented Mar 31, 2025

@archmoj I just figured out the source of those failing tests, it's because the Gravitas One font URL changed so the font download failed (see my updated PR comment).

@emilykl emilykl merged commit 40f2221 into master Mar 31, 2025
1 check passed
@emilykl emilykl deleted the update-map-baselines branch March 31, 2025 20:14
@@ -82,7 +89,7 @@ def download(repo, family, types) :
)

download(
'https://github.com/expo/google-fonts/blob/master/font-packages/gravitas-one/',
'https://github.com/expo/google-fonts/blob/main/font-packages/gravitas-one/400Regular/',
Copy link
Contributor

Choose a reason for hiding this comment

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

💯 🙏 Many thanks @emilykl
Switching from master to main broke these.
I suspect a similar problem would occur again.
I feel to avoid file changes (that occur on master/main branch of these repositories) it would be best to use permanent links for all these font files e.g.
https://github.com/expo/google-fonts/tree/cc65b43125023a2ad696eeec6ec10763c35ff588/font-packages/gravitas-one/400Regular instead of https://github.com/expo/google-fonts/blob/main/font-packages/gravitas-one/400Regular/.
Thank you!

archmoj reacted with heart emoji
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcjohnson alexcjohnson alexcjohnson approved these changes

@marthacryan marthacryan marthacryan approved these changes

@camdecoster camdecoster Awaiting requested review from camdecoster

@gvwilson gvwilson Awaiting requested review from gvwilson

+1 more reviewer

@archmoj archmoj archmoj left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
fix fixes something broken P1 needed for current cycle
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /