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

[build-tools] Remove fallback unversioned fingerprint diff #473

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wschurman
Copy link
Member

@wschurman wschurman commented Dec 3, 2024

Why

Follow-up to #460.

Since the output of the versioned diff fingerprint CLI is itself versioned (can change between fingerprint versions), we should no longer show the diff if it can't be computed in a versioned manner since the hardcoded local types may not be correct for all versions.

In theory most projects should now have a versioned fingerprint CLI (as of 52).

Closes ENG-14388.

How

Remove fallback code.

Test Plan

tsc

(this removes code and the tests for that removed code)

Copy link
Member Author

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

Copy link

linear bot commented Dec 3, 2024

@wschurman wschurman marked this pull request as ready for review December 3, 2024 01:54
@Kudo
Copy link

Kudo commented Dec 3, 2024

Since the output of the versioned diff fingerprint CLI is itself versioned (can change between fingerprint versions)

not having enough knowledge in eas build, i could miss the context here. could you share where the relevant code for the versioned output? especially for sdk 51 or lower projects, how do they diff fingerprint?

Copy link
Member Author

could you share where the relevant code for the versioned output?

The fingerprint:diff CLI command from the fingerprint package (as well as the diffFingerprints function) outputs a structure of the form FingerprintDiffItem, which is a structure that is defined in the versioned package. Thus, it is subject to change between SDK versions (it changed from SDK 51 to 52, for example). Rather than lock us into this format forever, it's preferable to only operate on this format in a versioned context. Thus, we only do the diff and parse the results in the versioned CLI.

especially for sdk 51 or lower projects, how do they diff fingerprint?

They don't. Fingerprint runtime version is still in beta so we don't need to support it in perpetuity, so now is as good a time as any to remove support to reduce attention cost of a beta feature.

@Kudo
Copy link

Kudo commented Dec 4, 2024

especially for sdk 51 or lower projects, how do they diff fingerprint?

They don't. Fingerprint runtime version is still in beta so we don't need to support it in perpetuity, so now is as good a time as any to remove support to reduce attention cost of a beta feature.

oh cool, that answers my question and is fair. thanks!

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