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

Namadillo: Checks for indexer and keychain compatibility #1449

Merged
merged 3 commits into from
Dec 27, 2024

Conversation

pedrorezende
Copy link
Contributor

Displays a warning if the version received by the keychain or indexer doesn't match the version defined on /compatibility.json file

image

@pedrorezende pedrorezende force-pushed the feat/check-compatibility branch from ba53629 to 8e1384e Compare December 27, 2024 14:57
Copy link
Contributor

@euharrison euharrison left a comment

Choose a reason for hiding this comment

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

Nice! ✅

target="_blank"
rel="nofollow noreferrer"
>
Firefox addons
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we omit the Firefox while the extension there doesn't exist yet?

Also, we could check for user agent and only provide the best match. What do you think?

const checkVersionsCompatible = (
currentVersion: string,
requiredVersion: string
): -1 | 1 | boolean => {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a personal taste, but don't you prefer an enum or const here? it's hard to read later on the conditions below

@pedrorezende pedrorezende merged commit 2a7cd74 into main Dec 27, 2024
10 checks passed
@pedrorezende pedrorezende deleted the feat/check-compatibility branch December 27, 2024 21:25
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.

2 participants