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

[eas-cli] compute fingerprint on each build #2663

Merged
merged 3 commits into from
Nov 6, 2024

Conversation

quinlanj
Copy link
Member

@quinlanj quinlanj commented Oct 30, 2024

Why

This PR computes a fingerprint for each build to support soft fingerprinting.

Testing

  • Verify fingerprint is computed in project with SDK 52
  • Verify fingerprint is computed in project with eas update
  • Verify workflow is still functional in projects without SDK 52 or eas update

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @quinlanj and the rest of your teammates on Graphite Graphite

@quinlanj quinlanj marked this pull request as ready for review October 30, 2024 01:50
Copy link

Subscribed to pull request

File Patterns Mentions
**/* @szdziedzic, @khamilowicz, @sjchmiela
packages/eas-cli/src/build/** @szdziedzic, @khamilowicz, @sjchmiela
packages/eas-cli/src/commands/update/** @EvanBacon, @byCedric, @wschurman

Generated by CodeMention

@quinlanj quinlanj marked this pull request as draft November 1, 2024 22:17
@quinlanj quinlanj force-pushed the 10-29-_eas-cli_compute_fingerprint_on_each_build branch from 6e6ea65 to 3c71784 Compare November 5, 2024 00:33
@quinlanj quinlanj marked this pull request as ready for review November 5, 2024 00:34
Copy link

github-actions bot commented Nov 5, 2024

Size Change: -2.41 kB (0%)

Total Size: 52.7 MB

Filename Size Change
./packages/eas-cli/dist/eas-linux-x64.tar.gz 52.7 MB -2.41 kB (0%)

compressed-size-action

Copy link

codecov bot commented Nov 5, 2024

Codecov Report

Attention: Patch coverage is 11.42857% with 31 lines in your changes missing coverage. Please review.

Project coverage is 52.87%. Comparing base (b039da0) to head (572a238).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/eas-cli/src/build/build.ts 5.27% 18 Missing ⚠️
packages/eas-cli/src/utils/fingerprintCli.ts 18.75% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2663      +/-   ##
==========================================
- Coverage   52.93%   52.87%   -0.05%     
==========================================
  Files         570      571       +1     
  Lines       21866    21898      +32     
  Branches     4306     4315       +9     
==========================================
+ Hits        11572    11576       +4     
- Misses      10260    10288      +28     
  Partials       34       34              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines 28 to 30
if (options.workflow === 'managed') {
fingerprintOptions.ignorePaths = ['android/**/*', 'ios/**/*'];
}
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR, but I wonder if it makes sense to hoist the workflow discrimination into the fingerprint library itself? (So that this stays consistent with https://github.com/expo/expo/blob/main/packages/expo-updates/utils/src/createFingerprintAsync.ts#L22). I don't foresee the logic changing, but it does seem fairly core to what fingerprint selects. Maybe make it an argument/option that, when provided, sets these ignore paths by default? cc @Kudo

Copy link

Choose a reason for hiding this comment

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

sounds good to me. maybe we can add an isCNG: boolean option.

packages/eas-cli/src/utils/fingerprintCli.ts Outdated Show resolved Hide resolved
packages/eas-cli/src/build/build.ts Outdated Show resolved Hide resolved
packages/eas-cli/src/build/metadata.ts Show resolved Hide resolved
@quinlanj quinlanj force-pushed the 10-29-_eas-cli_compute_fingerprint_on_each_build branch from 552d04f to 572a238 Compare November 6, 2024 00:22
Copy link

github-actions bot commented Nov 6, 2024

✅ Thank you for adding the changelog entry!

@quinlanj quinlanj merged commit 3f2dcc7 into main Nov 6, 2024
11 checks passed
@quinlanj quinlanj deleted the 10-29-_eas-cli_compute_fingerprint_on_each_build branch November 6, 2024 03:20
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.

4 participants