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

fix(iphone-codes): update BE mapping; remove unused method #82094

Merged
merged 4 commits into from
Dec 13, 2024

Conversation

armcknight
Copy link
Member

Update the mapping of Apple device hardware IDs to human readable names.

I was going to update https://github.com/getsentry/sentry/blob/master/static/app/constants/ios-device-list.tsx as was last done in #68762 along with this file, but I noticed this warning at the top:

// THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
// generated using scripts/extract-ios-device-names.ts as part of build step.
// the purpose of the script is to extract only the iOS information that Sentry cares about
// and discard the rest of the JSON so we do not end up bloating bundle size.

Question: Should that file have not been edited in that PR? Do we still run that script? If so, we need to put some kind of protection in place so that if the file is manually edited, a precommit hook or PR check flags/disallows it, because it wasn't obvious to me or any other reviewers on that last PR.

@armcknight armcknight added the Scope: Backend Automatically applied to PRs that change backend components label Dec 13, 2024
@armcknight armcknight requested a review from a team as a code owner December 13, 2024 19:06
@billyvg
Copy link
Member

billyvg commented Dec 13, 2024

@armcknight for the frontend you probably want to bump the npm package ios-device-list and then run the script to generate. doesn't seem like it regularly gets updated

@armcknight
Copy link
Member Author

armcknight commented Dec 13, 2024

@armcknight for the frontend you probably want to bump the npm package ios-device-list and then run the script to generate. doesn't seem like it regularly gets updated

@billyvg I'm having some discussion about that now. It seems that dependency isn't updated any longer: #68258

So I'll just manually update that file.

ETA: split that into a different pr: #82100

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Dec 13, 2024

This comment was marked as outdated.

@armcknight armcknight force-pushed the armcknight/fix/apple-device-identifiers branch from 5586bf5 to c4f9dbd Compare December 13, 2024 20:14
@armcknight armcknight changed the title fix(iphone-codes): update mapping; remove unused method fix(iphone-codes): update BE mapping; remove unused method Dec 13, 2024
@armcknight armcknight enabled auto-merge (squash) December 13, 2024 20:16
@armcknight armcknight merged commit f31695e into master Dec 13, 2024
49 checks passed
@armcknight armcknight deleted the armcknight/fix/apple-device-identifiers branch December 13, 2024 20:41
Copy link

codecov bot commented Dec 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #82094   +/-   ##
=======================================
  Coverage   80.36%   80.37%           
=======================================
  Files        7275     7275           
  Lines      321397   321394    -3     
  Branches    20962    20962           
=======================================
+ Hits       258303   258306    +3     
+ Misses      62683    62677    -6     
  Partials      411      411           

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants