[eas-cli] release fingerprint:compare #2821
Conversation
|
Size Change: +2.21 kB (0%) Total Size: 53.4 MB
|
|
❌ It looks like a changelog entry is missing for this PR. Add it manually to CHANGELOG.md. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2821 +/- ##
==========================================
- Coverage 52.48% 52.34% -0.14%
==========================================
Files 584 585 +1
Lines 22626 22701 +75
Branches 4464 4478 +14
==========================================
+ Hits 11873 11880 +7
- Misses 10718 10786 +68
Partials 35 35 ☔ View full report in Codecov by Sentry. |
|
Subscribed to pull request
Generated by CodeMention |
There was a problem hiding this comment.
Talked offline, but the summary of changes:
To avoid the platform inference, either:
a) if need to calculate fingerprint locally and need a platform, look for a linked build to get the platform from either from build-id flag or from builds linked to that fingerprint. If none is linked, throw error.
b) if need to calculate fingerprint locally, require build-id flag. get platform from build.
c) split this into two commands. I prefer this since the human-readable description of the allowed arg/flag combinations is simpler (see inline comment), but am happy to be overridden if others prefer something else:
1. fingerprint:diff hash1 hash2 - diffs two fingerprints by id/hash. no build/update arg for this one. instead it just simply diffs.
2. fingerprint:diff-local-directory-with-remote --build/--update - diffs local with fingerprint used by a build or an update. this satisfies the requirement of the spec in notion. doesn't take a hash. command name should be something else, but I called it diff-local-directory-with-remote to demonstrate intent.
d) maybe get rid of local-remote compatibility in its own command altogether and instead just include it in the build info (build:info, build:list)? maybe it's too slow for this. could be behind a flag on those commands, i.e. eas build:info <id> --compare-to-local-fingerprint returns {id: ...., isLocalFingerprintCompatible: bool}. I guess maybe not this option since the diff would be helpful.
Couple additional questions:
- What is the behavior when a debug fingerprint is compared to a non-debug fingerprint?
| @@ -54,70 +80,48 @@ export default class FingerprintCompare extends EasCommand { | |||
| nonInteractive, | |||
| withServerSideEnvironment: null, | |||
| }); | |||
| if (jsonFlag) { | |||
| if (json) { | |||
There was a problem hiding this comment.
I think this is probably not limited to this command, but I think the way it works is that we'd need to explicitly print to printJsonOnlyOutput in json mode?
| const displayName = await getDisplayNameForProjectIdAsync(graphqlClient, projectId); | ||
| const fingerprint = await FingerprintQuery.byHashAsync(graphqlClient, { | ||
| appId: projectId, | ||
| hash, |
There was a problem hiding this comment.
parallelize via Promise.all.
edit: or, even better, only fetch the displayName in the error case?
| return fingerprint; | ||
| } | ||
|
|
||
| async function getFingerprintAsync(fingerprintFragment: FingerprintFragment): Promise<Fingerprint> { |
There was a problem hiding this comment.
optional:
| async function getFingerprintAsync(fingerprintFragment: FingerprintFragment): Promise<Fingerprint> { | |
| async function getFingerprintFromFingerprintFragmentAsync(fingerprintFragment: FingerprintFragment): Promise<Fingerprint> { |
| filters: { hasFingerprint: true }, | ||
| }); | ||
| if (!buildId) { | ||
| throw new Error(); // exit, explanation already printed in prompt |
There was a problem hiding this comment.
| throw new Error(); // exit, explanation already printed in prompt | |
| throw new Error('Must select build with fingerprint for comparison.'); |
so the output will have both:
No fingerprints have been computed for builds of project ${projectDisplayName}.
Must select build with fingerprint for comparison.
(this matches the exit pattern for similar cases in other commands I believe)
| return { ignorePaths }; | ||
| } | ||
|
|
||
| function inferPlatformsFromSource(fingerprint: Fingerprint): AppPlatform[] { |
There was a problem hiding this comment.
talked about this offline, but I think if we can avoid this by limiting args or splitting out commands that'd be preferable. The main issue is that (I believe) it's possible to create a fingerprint for either platform or both that doesn't have either string being checked for (by using a specially-crafted .fingerprintIgnore or sourceSkips).
| static override hidden = true; | ||
| static override strict = false; | ||
|
|
||
| static override args = [ |
There was a problem hiding this comment.
while the command does the correct thing to error when an invalid combination of args and flags is passed in, I think it might be a bit tough to explain the different combinations that are allowed and what each combination does, and thus potentially cause a UX issue. See my note about maybe splitting them up into different commands based on use case so we can better require and explain different combinations of args.
Why
This PR releases
eas fingerprint:comparefrom the hidden state. It also adds support for the following scenarios:eas fingerprint:compare [HASH1]: If provided alone, HASH1 is compared against the current project's fingerprint.eas fingerprint:compare [HASH1] [HASH2]: If two hashes are provided, HASH1 is compared against HASH2.More details: https://www.notion.so/expo/Soft-fingerprinting-116e5b5735248071951cddcab32ae43d
How
hash1andhash2Test Plan
Test the following scenarios:
EXPO_STAGING=1 ~/Documents/eas-cli/packages/eas-cli/bin/run fingerprint:compare --build-id 1aa82a8d-6fed-4c4c-ab93-ef07d0c7ca97EXPO_STAGING=1 ~/Documents/eas-cli/packages/eas-cli/bin/run fingerprint:compare f0d6a916e73f401d428e6e006e07b12453317ba2 c71a7d475aa6f75291bc93cd74aef395c3c94eeeEXPO_STAGING=1 ~/Documents/eas-cli/packages/eas-cli/bin/run fingerprint:compare c71a7d475aa6f75291bc93cd74aef395c3c94eee